Bugzilla

In the review Gijs asked me to check content-disposition too.
Apparently NS_GetContentDispositionFromHeader reads the content-disposition header through nsMIMEHeaderParamImpl::DoParameterInternal that pretty much stops at the first null, so we skip anything after a null: https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/netwerk/mime/nsMIMEHeaderParamImpl.cpp#465 

I was curious to check what Chrome does in this case, and to my surprise I got an ERR_INVALID_HTTP_REPONSE error page. Digging around I found https://bugs.chromium.org/p/chromium/issues/detail?id=927364 where developers says Chrome by choice returns an invalid http response error if the headers contain null characters. And they also say Firefox developers were contacted to ask whether interested into doing the same.
I tried to find a bug in Bugzilla about that, but I couldn't.

Basically if we'd error out on headers containing null characters, we 'd not require any handling here, otherwise I'm not sure what outcome to expect, should we change the parsing code to replace nulls with _ in certain cases? It seems a scarier path to take.

In the review Gijs asked me to check content-disposition too.
Apparently NS_GetContentDispositionFromHeader reads the content-disposition header through nsMIMEHeaderParamImpl::DoParameterInternal that pretty much stops at the first null, so we skip anything after a null: https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/netwerk/mime/nsMIMEHeaderParamImpl.cpp#465 

I was curious to check what Chrome does in this case, and to my surprise I got an ERR_INVALID_HTTP_REPONSE error page. Digging around I found https://bugs.chromium.org/p/chromium/issues/detail?id=927364 where developers say Chrome by choice returns an invalid http response error if the headers contain null characters. And they also say Firefox developers were contacted to ask whether interested into doing the same.
I tried to find a bug in Bugzilla about that, but I couldn't.

Basically if we'd error out on headers containing null characters, we 'd not require any handling here, otherwise I'm not sure what outcome to expect, should we change the parsing code to replace nulls with _ in certain cases? It seems a scarier path to take.