Uh oh!
There was an error while loading. Please reload this page.
fix: skip http proxy added header - #7622
Merged
kenjis merged 4 commits intoJun 29, 2023
Merged
Conversation
kenjis
commented
Jun 26, 2023
Member
What is the HTTP proxy problem? |
ContributorAuthor
@kenjis I added also test for it but I have precommit problem with memory, basically it's a problem in one of our deployment where Linux machine is communicating with internet via HTTP Proxy. In this enviroment it's Fortinet HTTP Proxy. Our setup is as follow we have Red Hat Enterprise with Docker. CodeIgniter4 docker image is running on Alpine OS. We have configured HTTP/1.1 200 Connection established
Proxy-Agent: Fortinet-Proxy/1.0
HTTP/2 200
date: Mon, 26 Jun 2023 07:44:50 GMT
content-type: text/css; charset=utf-8
access-control-allow-origin: *
access-control-expose-headers: *
timing-allow-origin: *
cache-control: public, max-age=31536000, s-maxage=31536000, immutable
cross-origin-resource-policy: cross-origin
x-content-type-options: nosniff
strict-transport-security: max-age=31536000; includeSubDomains; preload
x-jsd-version: 4.14.3
x-jsd-version-type: version
etag: W/"23529-0Aplim6MlPCHdtldoUGfLqonpGQ"
x-served-by: cache-fra-eddf8230023-FRA, cache-jnb7021-JNB
x-cache: HIT, MISS
vary: Accept-Encoding
alt-svc: h3=":443"; ma=86400
cf-cache-status: HIT
age: 2190
report-to: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=3zBXBy6F9ytYZdUmO4cw7tBl%2Fys3wmyumlUD0Ht%2FdIa%2FXr4NjHNOxniwgYTjn4iPV2%2FsqdcQKwIEdNCQ5lgMQU8eGVeJsa2%2F%2BTyyWnSiT4Akd7EhFXaPIRHLoiiVqZYmgAw%3D"}],"group":"cf-nel","max_age":604800}
nel: {"success_fraction":0.01,"report_to":"cf-nel","max_age":604800}
server: cloudflare
cf-ray: 7dd3d3c99b6c27c0-PRG
.swagger-ui{color:#3b4151;font-family:sans-serif/*! normalize.css v7.0.0and after that body includes in assets, I believe this is the fastest way how to fix it, there is already continued header skiped. |
michalsn
approved these changes
Jun 26, 2023
kenjis
reviewed
Jun 26, 2023
| $output = substr($output, strpos($output, $breakString) + 4); | ||
| } | ||
| if (strpos($output, 'HTTP/1.1 200 Connection established') === 0) { |
Member
There was a problem hiding this comment.
It may be "Connection Established".
Is it always "HTTP/1.1" ?
ContributorAuthor
There was a problem hiding this comment.
@kenjis You are right, I'm studying now whole concept behind this and it seems that there should be different headers.
The original tunnelling spec required the proxy to return a 200 Connection Established to the client once it had successfully connected to the server, which is what we're seeing. This is potentially followed by more headers, and then an empty line, before the connection becomes transparent and we get the actual response from the server. So, we get two sets of headers. RFC 2817 relaxes this, and allows any 2xx response to a CONNECT request.
Some very late clarification. When you connect to an SSL/TLS server through a proxy, the proxy establishes a tunnel, using HTTP CONNECT. This is covered in RFC2817 and the expired tunneling spec, and not RFC261.
When making an https request through a proxy, a CONNECT request is of
course made to the proxy before the SSL connection can be made with the
remote server.
Currently, the proxy's response to the CONNECT is written to the libcurl
callback function like the rest of the response, such that it appears
before the remote server's response headers. So, for instance, to my
client code, the response looks like this:
HTTP/1.1 200 Connection established <-- proxy's response Proxy-Agent: Fortinet-Proxy/1.0HTTP/2 200 <-- remote server's response date: Mon, 26 Jun 2023 07:44:50 GMT etc some proxies are rather minimalist and only give:
HTTP/1.0 200 <-- proxy's response HTTP/1.0 200 OK <-- remote server's response Date: Tue, 04 Oct 2005 20:17:22 GMT etc The best method I have thought of for detecting whether the response
contains a header like this (with a proxy's CONNECT response first) is
to set a header callback and see if I get an empty header line in the
middle of the headers. I haven't actually tested this solution yet.
Member
On Start Squid proxy server. Controller: $client = \Config\Services::curlrequest();
$response = $client->request(
'GET',
'https://forum.codeigniter.com/syndication.php?limit=1',
[]
);
dd($response);Response body without Proxy: Start Response body with Proxy: |
Member
The original |
jozefrebjak
commented
Jun 27, 2023
ContributorAuthor
I believe everything else than |
kenjis
commented
Jun 28, 2023
Member
I implemented a request option $client = \Config\Services::curlrequest();
$response = $client->request(
'GET',
'https://forum.codeigniter.com/syndication.php?limit=1',
[
'proxy' => 'http://localhost:3128',
]
);
dd($response);And the response body is the same as before this PR. |
kenjis
approved these changes
Jun 28, 2023
kenjis
left a comment
Member
There was a problem hiding this comment.
This is not perfect, but works for now.
kenjis
commented
Jun 29, 2023
Member
@jozefrebjak Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request addresses the HTTP proxy problem by adding a new conditional statement to the code. Specifically, the code checks if the output string begins with
HTTP/1.1 200 Connection established.For example, when the output string contains the following header:
If it does, then the code uses
substrfunction to remove the initial part of the string up to the specified$breakString, which helps in handling the HTTP proxy issue. The added code segment is as follows:This change ensures that the HTTP proxy connection is handled properly and the code runs smoothly without any errors.
Checklist: