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

Mismatched Host header using proxy in [email protected] #918

Closed
LouisSung opened this issue Jan 7, 2021 · 7 comments · Fixed by #919
Closed

Mismatched Host header using proxy in [email protected] #918

LouisSung opened this issue Jan 7, 2021 · 7 comments · Fixed by #919

Comments

@LouisSung
Copy link
Contributor

Describe the bug
As of [email protected], the "azure-devops-node-api": "^8.1.1" is used.
However, some issues are fixed between the latest [email protected] and [email protected], such as microsoft/typed-rest-client#163

To Reproduce
All API calls are effected by this bug

Expected behavior
Should update the dependency as the latest version, at least 10.1.0

Related issues

  1. I soon raised a PR http_client: prevent domain fronting (Host mismatch) when using proxy nodejs/node#36805 to fix the Host header mismatch issue at the Node.js runtime level (root cause)
  2. However, by communicating with Microsoft support team today, this issue has been addressed in Wrong Host header prevents connections through proxy microsoft/typed-rest-client#163 and Bad Host Header of CONNECT request koichik/node-tunnel#29 that fixed by Updated tunnel package version to 0.0.6 microsoft/typed-rest-client#195 and deployed since [email protected], microsoft/[email protected] and microsoft/[email protected].
@rfennell
Copy link
Owner

rfennell commented Jan 7, 2021

I will get sorted ASAP

@rfennell rfennell self-assigned this Jan 7, 2021
rfennell added a commit that referenced this issue Jan 7, 2021
rfennell added a commit that referenced this issue Jan 7, 2021
@rfennell
Copy link
Owner

rfennell commented Jan 7, 2021

It has been shipped in 3.40.10

@LouisSung
Copy link
Contributor Author

I'll check it with packet analyzer tomorrow
Thank you~

@LouisSung
Copy link
Contributor Author

Sorry about causing the #928 _(;3

We also met the error when I check yesterday, but I'm blocked from our company's firewall and cannot reply or comment...
I have to clone the source code, revert, build, and push our own version to market and then replace the task for all the taskgroups we have; otherwise, it impacts hundreds of our release pipelines

Is there any good mechanisms to keep the stable version or delay the deployment?
Since users cannot force specify or downgrade the minor version in Azure DevOps...

@rfennell
Copy link
Owner

rfennell commented Jan 9, 2021

#928 was going to happen in the end when I got around to a library upgrade, hopefully 3.41.x fixes it as it is a known issue. It is a bit concerning that all my test pass 100% and my own companies usage is fine with both 3.40.x and 3.41.x, I am obviously missing some edge case, just not sure what it is yet.

I seems #928 is now addressed, but of not I will probably be rolling back the library in the 3.x branch and make the change on 4.x to allow people to swap when they choose

@LouisSung
Copy link
Contributor Author

LouisSung commented Jan 13, 2021

Oh, I miss the reply again...
The #929 shipped as 3.41.31 also fixes our problems, so I think it's safe to close #928.

I think what I learned from this issue is that CI may covers most of the logic parts but maybe not the configurations or environment parts.
For example, the CI pipeline run using Microsoft's public agent; therefore, the issue #928 was not detected due to it's specific to on-premises agents.
(also issues like empty work items, the default configuration changes but the CI environment doesn't reflect that situation)

I know it's not anyone's fault and you also response to issues very very fast, but Azure provides no ways to revert version manually (I've even tried to use API to change version from 3.* to 3.40.10 but it turns out invalid versioning error message).

Not to say that providing stable or deferred updated version is essential but it worth considering if the change may impact a lot :)

But still, I don't feel providing another versions like 4.* would be a good practice in the semantic versioning point of view...
We have to self-host our own version afterward to keep things stable because we apply this task to all of our release pipelines and therefore couldn't afford any failures _(;3

@rfennell
Copy link
Owner

Incrementing the major version cause problems of it's own, basically most people don't upgrade and support become harder due to parallel versions. Hence, I only 'up' the major number when there is major breaking change or more commonly major rewrite, and the minor number for all other non breaking changes/bug fixes. Noting that I always add defaults to keep functionality the same for existing users where possible,

This issue should not have been a breaking change. However, it has shown I do need to alter my release process, especially if there a library updates, to do extra testing on premise agents and even Azure DevOps Server.

@LouisSung LouisSung changed the title Wrong Host header issue using [email protected] with proxy Mismatched Host header using proxy in [email protected] Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants