wrapTillBound in Network/Wai/Parse.hs incorrectly counting max chunk size (e.g. file sizes towards max parameter size)

[moll]

Hey, all!

Thanks for maintaining Wai and the related modules!

I'm seeing some odd behavior where Wai Parse throws PayloadTooLarge when it shouldn't. That is, it seems to be counting multipart file parts' sizes towards the max parameter size (prboMaxParmsSize).

Reading the code, I suspect this is due to

wai/wai-extra/Network/Wai/Parse.hs

Lines 741 to 756 in 53da27d

	go ref sref = do
	state <- readIORef ref
	case state of
	WTBDone _ -> return S.empty
	WTBWorking front -> do
	bs <- readSource src
	cur <- atomicModifyIORef' sref $ \cur ->
	let new = cur + fromIntegral (S.length bs) in (new, new)
	case max' of
	Just max'' | cur > max'' -> E.throwIO PayloadTooLarge
	_ -> return ()
	if S.null bs
	then do
	writeIORef ref $ WTBDone False
	return $ front bs
	else push $ front bs

, where it reads a chunk and checks its size before looking for a boundary. It's only in the push function where boundary seeking happens. If a multipart requests comes in with a non-file parameter first and the entire read chunk is larger than the permitted parms size, it errors out, even if the particular parameter its reading is well below the permitted size. The same should technically happen backwards, too --- file first, param later. In other words, the size check should happen after a boundary is found.

If I'm correct, I suppose this hasn't come up in the years before due to the default max params size (64kB) being larger than a read chunk in production situations. Even I initially saw this in tests, where I suspect the Network.Wai.Test module jams the entire body into one chunk somehow. In non-test settings I tracked it down by setting the max parameters size to small numbers (e.g. 256).

Speaking of PayloadTooLarge, perhaps it'd make sense to replace the Wai's general PayloadTooLarge error with a form-specific construtor in RequestParseException. I'm very strict in handling all errors and right now the Wai Parse module's error cases aren't totally expressible with RequestParseException. PayloadTooLarge also loses information about whether it was a param that was too large or a file. Equally, it'd be nice if the error(s) included the max sizes, too, for better end-user error messages. Right now they contain the actual sizes.

Thanks and cheers!