Suggested patch in order to solve problem of Safari and HTTP/2 error

[ubrmensch]

Hello,

We have been using this library as part of our project in conjunction with phpwkhtmltopdf library.

We recently migrated to a new server, and suddenly the PDF download stopped working over https. After a lot of digging, we realized that the old server was serving https over HTTP/1.0. The new server was serving https over HTTP/2.

As it turns out, the Content-Length header ALSO causes network issues in the latest Safari (which now supports HTTP/2).

We patched the File.php line were the bug is mentioned from this:

    // #84: Content-Length leads to "network connection was lost" on iOS
    $isIOS = preg_match('/i(phone|pad|pod|safari)/i', $_SERVER['HTTP_USER_AGENT']);

to this:

    // #84: Content-Length leads to "network connection was lost" on iOS AND latest Safari.
    $isIOS = preg_match('/(phone|pad|pod|safari)/i', $_SERVER['HTTP_USER_AGENT']);

We removed the first "i" after the preg_match opening bracket because it seemed like it was actually causing the expression to never match. Not sure if that's something that has been dropped in PHP or not. However, once we that simple change, the php downloads worked fine on Safari over https / HTTP/2. From my understanding, all modern servers will only server HTTP/2 over HTTPS so this should not be a problem on regular HTTP/1.0, which is probably the case for most people.

[mikehaertl]

Hmm, it seems you already use some patched version. We don't have any safari in the regular expression:

https://github.com/mikehaertl/php-tmpfile/blob/1.1.4/src/File.php#L85

Regarding the issue with HTTP/2 I have to do some research if this can be fixed somehow.

Related: mikehaertl/php-pdftk#84

[mikehaertl]

This is really a tough problem: I can not think of a way to reliably detect HTTP/2 requests from PHP.

While there is $_SERVER['SERVER_PROTOCOL'] this is not reliable, because it's a very common scenario that your app sits behind a reverse proxy e.g. Nginx. The value there only reports the backend upstream protocol. Nginx does not support HTTP/2 for upstream requests. So even if your client connection uses HTTP/2 you still would get HTTP/1.0 as SERVER_PROTOCOL.

And even a configuration flag would not help, because if you disable Content-Length completely you would get a problem with all requests from HTTP/1.x.

If someone has an idea let me know. For now I can't think of any.

[mikehaertl]

One proper solution would probably be this:

• We leave away the Content-Length header if SERVER_PROTOCOL is HTTP/2
• In the other case where a proxy connects with HTTP/1.x but uses HTTP/2 to the client, the proxy is responsible to remove/fix the Content-Length header.

But this would require a smart proxy. It can probably be done with Nginx - but still it's not a very convenient solution.

[ubrmensch]

I modified that line of code and added safari to the regular expression. The first “i” in that expression seemed to always return false regardless of the string. We removed it. Is there a reason it was originally this: $isIOS = preg_match('/i(phone|pad|pod)/i', $userAgent); …and not this $isIOS = preg_match('/(phone|pad|pod)/i', $userAgent); …also, just to be clear, this is only a Safari issue. All other browsers we tested safely ignore Content-Length over HTTP/2.

…

On Jul 17, 2020, at 2:36 AM, Michael Härtl ***@***.***> wrote: Hmm, it seems you already use some patched version. We don't have any safari in the regular expression: https://github.com/mikehaertl/php-tmpfile/blob/1.1.4/src/File.php#L85 <https://github.com/mikehaertl/php-tmpfile/blob/1.1.4/src/File.php#L85> Regarding the issue with HTTP/2 I have to do some research if this can be fixed somehow. Related: mikehaertl/php-pdftk#84 <mikehaertl/php-pdftk#84> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQS3YZVIP2RNBYBAANDR37WOVANCNFSM4O463LHA>.

[mikehaertl]

@ubrmensch The i there is ok, because we want to test for iphone, ipod, ipad. The correct expression should then rather be /(iphone|ipad|ipod|safari)/i. But this makes me wonder if this has changed. If you check the issue linked above, the original report was from a safari user, too. And we fixed it by checking for iphone, ipad, ...

also, just to be clear, this is only a Safari issue. All other browsers we tested safely ignore Content-Length over HTTP/2.

Hmm, I'm not sure. If you check the other issue the last commenter seems to confirm that HTTP/2 is affected in general. I still think, this needs a better fix.

[ubrmensch]

All three use Webkit. It is not likely that they previous thread was related to Safari because Safari only recently started to support HTTP/2, which is why if you google this problem on the Internet, it seems to have become a problem with other software libraries too. I think checking for HTTP/2 even if it’s not reliable is still a good idea anyways. One other piece of information that might help. I have a development server that runs pure Apache but runs both http and https over HTTP/1.1. When I test our modified code to drop the Content-Length if Safari is detected in the user agent, the download still works. I am using version Version 13.1.2 (15609.3.5.1.3). It may be the case that in older versions of safari, you need Content-Length or else when using HTTP/1.1 & https...or else it will break. Regardless, I like your idea of checking the$_SERVER['SERVER_PROTOCOL’] and I’m not clear why that would be unreliable. If it is, perhaps another check you could make is: I still think the FIRST “i” in the regular expression currently in the code base needs to be removed. We removed it in our patch because it keep returning false regardless.

…

On Jul 17, 2020, at 5:08 AM, Michael Härtl ***@***.***> wrote: @ubrmensch <https://github.com/ubrmensch> The i there is ok, because we want to test for iphone, ipod, ipad. The correct expression should then rather be /(iphone|ipad|ipod|safari)/i. But this makes me wonder if this has changed. If you check the issue linked above, the original report was from a safari user, too. And we fixed it by checking for iphone, ipad, ... also, just to be clear, this is only a Safari issue. All other browsers we tested safely ignore Content-Length over HTTP/2. Hmm, I'm not sure. If you check the other issue the last commenter seems to confirm that HTTP/2 is affected in general. I still think, this needs a better fix. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQS6MT24X4AZULRORGDR4AIIHANCNFSM4O463LHA>.

[mikehaertl]

It is not likely that they previous thread was related to Safari because Safari only recently started to support HTTP/2,

Have you checked the screenshots on top of the other issue? It shows "Safari". Oh, and also notice that this is on mobile. Maybe you talk about safari on desktop? As explained before iphone, ipad and ipod are the strings that we want to test for in the other issue - and this worked, so the "i" is correct there.

I will consider adding a check for HTTP/2.

[ubrmensch]

sorry…missed a line in my last reply: Regardless, I like your idea of checking the$_SERVER['SERVER_PROTOCOL’] and I’m not clear why that would be unreliable. If it is, perhaps another check you could make is: if user agent is Safari AND if using https port, then drop the Content-Length. I guess here the assumption would be that older Safari browsers (of which I do not have access to) should be find with dropping Content-Length if https over http/1.1 and http/2.0

…

On Jul 17, 2020, at 5:36 AM, Peter Ilias ***@***.***> wrote: All three use Webkit. It is not likely that they previous thread was related to Safari because Safari only recently started to support HTTP/2, which is why if you google this problem on the Internet, it seems to have become a problem with other software libraries too. I think checking for HTTP/2 even if it’s not reliable is still a good idea anyways. One other piece of information that might help. I have a development server that runs pure Apache but runs both http and https over HTTP/1.1. When I test our modified code to drop the Content-Length if Safari is detected in the user agent, the download still works. I am using version Version 13.1.2 (15609.3.5.1.3). It may be the case that in older versions of safari, you need Content-Length or else when using HTTP/1.1 & https...or else it will break. Regardless, I like your idea of checking the$_SERVER['SERVER_PROTOCOL’] and I’m not clear why that would be unreliable. If it is, perhaps another check you could make is: I still think the FIRST “i” in the regular expression currently in the code base needs to be removed. We removed it in our patch because it keep returning false regardless. > On Jul 17, 2020, at 5:08 AM, Michael Härtl ***@***.*** ***@***.***>> wrote: > > > @ubrmensch <https://github.com/ubrmensch> The i there is ok, because we want to test for iphone, ipod, ipad. The correct expression should then rather be /(iphone|ipad|ipod|safari)/i. But this makes me wonder if this has changed. If you check the issue linked above, the original report was from a safari user, too. And we fixed it by checking for iphone, ipad, ... > > also, just to be clear, this is only a Safari issue. All other browsers we tested safely ignore Content-Length over HTTP/2. > > Hmm, I'm not sure. If you check the other issue the last commenter seems to confirm that HTTP/2 is affected in general. I still think, this needs a better fix. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQS6MT24X4AZULRORGDR4AIIHANCNFSM4O463LHA>. >

[ubrmensch]

Yes, I should have been clearer, I was talking about Desktop version of Safari. I see what you mean regarding the first i…my only point is that if you patch it to include safari (like we did), the expression will not work because it will try to match isafari instead of just safari. Perhaps it could be modified to drop the i, or put the i in front of each iOS device: preg_match(‘/(iphone|ipad|ipod|safari)/i', $userAgent) Otherwise, if you don’t patch it to detect desktop Safari, it works fine. thx.

…

On Jul 17, 2020, at 5:42 AM, Michael Härtl ***@***.***> wrote: It is not likely that they previous thread was related to Safari because Safari only recently started to support HTTP/2, Have you checked the screenshots on top of the other issue? It shows "Safari". Oh, and also notice that this is on mobile. Maybe you talk about safari on desktop? As explained before iphone, ipad and ipod are the strings that we want to test for in the other issue - and this worked, so the "i" is correct there. I will consider adding a check for HTTP/2. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQRX23W3GNAQUAXT2LTR4AMI7ANCNFSM4O463LHA>.

[mikehaertl]

One more question: Do you use mod_deflate in Apache? Because this would definitely break Content-Length. If so, you could try to disable mod_deflate for the download URL.

Apart from that I still see no good solution. We can not just disable Content-Length for Safari, because this would turn off the progress bar when HTTP/1.1 is used.

[ubrmensch]

I will have a look into that later today. I see your dilemma though. The patch we made will work specifically on our server because we know with certainty how the web server is serving https requests. Just out of curiosity, you mentioned a few times PHP cannot be reliably trusted to detect whether HTTP/2 is being used in the response. Why exactly is $_SERVER['SERVER_PROTOCOL’] not reliable? I have to admit, I only have a cursory knowledge of PHP architecture but I thought your original suggestion, including perhaps checking also that the request is over HTTPS, would be sufficient. Do you have a Patreon account? I feel like we should be contributing something for using your library. It definitely saved us quite a bit of time in not having to write our own wrapper class for wkhtmltopdf. Thx.

…

On Jul 17, 2020, at 9:10 AM, Michael Härtl ***@***.***> wrote: One more question: Do you use mod_deflate in Apache? Because this would definitely break Content-Length. If so, you could try to disable mod_deflate for the download URL. Apart from that I still see no good solution. We can not just disable Content-Length for Safari, because this would turn off the progress bar when HTTP/1.1 is used. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQSWG5VKXRYEQQII2WDR4BES7ANCNFSM4O463LHA>.

[mikehaertl]

It's not PHP specific. Just think about a setup like this:

[Client/User Browser] ----(HTTP/2)---->  [Proxy (e.g. Loadbalancer, Nginx, ...)] ----(HTTP/1.1)---> [App (e.g. docker container)]

So a client connects to your website with HTTP/2. A proxy relays the request to your app (which could even run on the same host) and uses HTTP/1.1 for that upstream request. This is a very common setup for example if your app runs in a docker container. Nginx can not use HTTP/2 for the backend connection.

So your app has no idea what protocol the client really uses - it only "sees" HTTP/1.1 while the client still "sees" HTTP/2. That's why $_SERVER['SERVER_PROTOCOL'] can not be trusted here.

About patreon: I don't have an account, but thanks for the offer. I'm already happy if people find my code useful.

[ubrmensch]

Admittedly, I don’t know a lot about nginx or the way browsers know in advance whether to make an HTTP/1.1 OR HTTP/2.0 request…and whether you can receive back a HTTP/1.1 response from a HTTP/2 request…which I’m assuming what your concern is bellow. However, I did this test on my local machine (which runs Apache without nginx and only runs HTTP/1.1) using Firefox…. 1. sent a request http://localhost/test.php <http://localhost/test.php> (request header had HTTP 1.1) and got response in HTTP 1.1. 2. sent a request https://localhost/test.php <https://localhost/test.php> (request header HTTP 1.1) and got response back in HTTP 1.1. Then I ran the exact same test on the new server with nginx + Apach: 3. sent a request http://localhost/test.php <http://localhost/test.php> (request header had HTTP 1.1) and got response in HTTP 1.1. 4. sent a request https://localhost/test.php <https://localhost/test.php> (request header had HTTP 2) and got response back in HTTP 1.1. Question…how did the browser know in steps 2 and 4 (which both went to https) to make the request header HTTP 1.1 in the first case, and HTTP 2 in the second case. It’s almost as if the browser knew that the second server supports HTTP 2 and thus put that in the request header whereas the exact same browser made the http request on the dev server using HTTP 1.1. Is there some kind of handshaking before the request is made? Anyways, I’ve probably spent too much of your time on this. Last thought…because it sounds like there is no perfect solution. If I were a Safari user on desktop, I would rather live with not having the download progress indicator than not having a download at all…which is the case now when sending a pdf file over https on a server that supports HTTP/2. And since this only started happening when we rebuilt our Amazon EC2 server this month, I’m guessing a lot more servers will be coming online with HTTP/2 enabled by default on HTTPS. So again, IMHO, if your code queried for HTTPS port in PHP, AND ipod/ipad/safari are present in the user agent, drop the Content-Length. Otherwise, the user would be left with no download at all…which was the case for us. Cheers.

…

On Jul 17, 2020, at 12:53 PM, Michael Härtl ***@***.***> wrote: It's not PHP specific. Just think about a setup like this: [Client/User Browser] ----(HTTP/2)----> [Proxy (e.g. Loadbalancer, Nginx, ...)] ----(HTTP/1.1)---> [App (e.g. docker container)] So a client connects to your website with HTTP/2. A proxy relays the request to your app (which could even run on the same host) and uses HTTP/1.1 for that upstream request. This is a very common setup for example if your app runs in a docker container. Nginx can not use HTTP/2 for the backend connection. So your app has no idea what protocol the client really uses - it only "sees" HTTP/1.1 while the client still "sees" HTTP/2. That's why $_SERVER['SERVER_PROTOCOL'] can not be trusted here. About patreon: I don't have an account, but thanks for the offer. I'm already happy if people find my code useful. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIYTIQTRR6MZ7BH7HCJPKJLR4B6XFANCNFSM4O463LHA>.

[mikehaertl]

Sorry, we're getting ab it off topic and your scenario is not what I meant. The problem is really with a proxy setup and I suggest you do some reasearch if that's unclear, e.g. here: https://en.wikipedia.org/wiki/Reverse_proxy.

IMO it has not really to do with HTTPS but rather with HTTP/2 and certain browsers. And I'm still curious if you use mod_deflate with apache or the gzip module in Nginx.