Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Rename and clarify the `body.collect(upTo:)` API which gets used in an insecure way all the time

Aperta
  • Qualcosa non va?
#745 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
25/100
Tipo di issue
Funzionalità
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
swift
Ambito
backend-api-design

Direzione di ricerca

Inizia individuando l’API body.collect(upTo:) e la relativa documentazione, quindi esamina i punti in cui viene attualmente chiamata e i test esistenti. Usa gli obiettivi e gli esempi dell’issue per definire un’API a memoria limitata facilmente individuabile, un percorso unsafe esplicito e una documentazione che spieghi quando è preferibile lo streaming; il lavoro è completo quando il design è concordato e coperto dai test.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

We see code similar to this a lot

let expectedBytes = response.headers.first(name: "content-length").flatMap(Int.init) ?? 1024 * 1024
return Response(status: response.status, buffer: try await response.body.collect(upTo: expectedBytes))

The above code doesn't make any sense because it essentially says

Read as many bytes as you want into memory

This of course can lead to very bad scenarios because the content-length is controlled by the attacker. Furthermore, it's a lot of code to spell the ~equivalent of

return Response(status: response.status, buffer: try await response.body.collect(upTo: Int.max))

And just to be clear, we do not have to check content-length here. The content length is correct, or else NIO would've spotted that and sent an error instead (like illegal EOF condition if it ended prematurely or so).

But let's get to fixing this:

Goals:

  • Reasonable (fixed-sized limit) code should look good and be discoverable without reading much documentation
  • Unreasonable code should be possible but awkward using some word that conveys the danger of what's happening
  • Not overcomplicated

Ideas:

  • Put a reasonable default limit (e.g. 4MB) on there. So body.collect() would be safe
  • Make it clear in the docs that streaming should always be preferred. For example to save something to disk or proxy it, collect(...) should not be used. Only for JSON/protobuf/image/... decoding that needs to happen in memory we should use it
  • Instead of Int use a new type that can be constructed with integer literals or (and thanks to @hamzahrmalik for the idea) using some .init(watchOutWhatYourAreDoingIsDangerous: Int)

Maybe

  • body.collectIntoMemory() -- safe: up to 4 MB
  • body.collectIntoMemory(maximumSize: .megabytes(16)) -- safe: up to 16 MB
  • body.collectIntoMemory(maximumSize: .bytes(contentLength)) -- compiler error: expected type BodySize, actual type Int
  • body.collectIntoMemory(maximumSize: .bytes(BodySize(dangerousCalculatedSizeInBytes: contentLength)) -- unsafe but compiles
Lingua principale
Swift
Stelle
1.1k
Fork
156
Merge medio
1h 54m
PR unite (30g)
4

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di swift-server/async-http-client

Tutte le issue di swift-server/async-http-client

Issue simili

Altre issue su Swift

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.