Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPUpgrade send headers with specified capitalization #3430

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

Fangliding
Copy link
Member

昨晚上看到就知道怎么搞了 感觉没必要就没动 没想到第二天起来看到还写了个这么复杂的玩意的
个人觉得还是有一点点必要维持可读性的()

@Fangliding
Copy link
Member Author

好像revert了不该revert的东西 等一下下()

@Fangliding Fangliding force-pushed the httpupgrade-header branch from ede8511 to 34efa1d Compare June 6, 2024 05:56
@mmmray
Copy link
Collaborator

mmmray commented Jun 6, 2024

thank you, in general i agree. i did not intend to write such a complex code originally.

i did not know it's possible to insert into the map directly, that's cool. but there is now another issue. if i have a config like this:

        "httpupgradeSettings": {
          "path": "/hu",
          "headers": {
            "host": "example.com"
          }
        }

this version of xray will send two host headers Host and host

this may seem like a minor issue, but i think existing configs like this could break. i think it can be fixed using an explicit hasHostHeader variable...

about simplicity... originally i wanted to remove all http.Request. in my mind net/http is very complex to understand, so this is what i was trying to remove here. if no users report bugs in the next version of xray, i can open a PR to restore the state of commit 097990926fb6ccd3c50c2bd798ef856d9208fb7f in #3427. this is also simpler, what do you think?

@Fangliding
Copy link
Member Author

Fangliding commented Jun 6, 2024

this version of xray will send two host headers Host and host

It has a pre-check, I just need 2 lines to fix it.

i can open a PR to restore the state of commit 0979909

No need. I'm just saying it is unnecessary to worry about the capitalization of the header. If there's a simple way to implement it, let's just keep it

@mikeesierrah
Copy link

Thanks @Fangliding @mmmray

also @Fangliding one thing to share :

IRGFW throttles and blocks all unknown sni's it kinda went MAD in that sense ( Cant understand --> block ) - and you are overestimating abilities of current IRGFW in fingerprinting notls connections

Still i understand XRAY is not specific to a single country its for everyone and you want something that everyone can use - but its good to be less-fingerprintable with notls

@yuhan6665 yuhan6665 changed the title HTTPUpgrade发送确切的标头大小写 HTTPUpgrade send headers with specified capitalization Jun 6, 2024
@yuhan6665 yuhan6665 merged commit fd7c44a into XTLS:main Jun 6, 2024
34 checks passed
@yuhan6665
Copy link
Member

Thanks all!

@Fangliding Fangliding deleted the httpupgrade-header branch June 6, 2024 17:43
Fangliding added a commit that referenced this pull request Jun 7, 2024
* Fix HTTPUpgrade header capitalization

* Chore

* Remove excess host headers

Chore : change httpupgrade header "upgrade" to "Upgrade" #3435
leninalive pushed a commit to amnezia-vpn/amnezia-xray-core that referenced this pull request Oct 29, 2024
* Fix HTTPUpgrade header capitalization

* Chore

* Remove excess host headers

Chore : change httpupgrade header "upgrade" to "Upgrade" XTLS#3435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants