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

fix: proxy authorization type Basic #22471

Conversation

Buluc-Celik-Ozbul
Copy link
Contributor

@Buluc-Celik-Ozbul Buluc-Celik-Ozbul commented Jun 23, 2022

User facing changelog

The Proxy-Authorization header when present with basic authorization will read "Basic" with a capital "B".

Additional details

When I set up a proxy with username and password, the Proxy-Authorization header is created with basic type but it says "basic" with a small "b" and this fails with some proxies. The Proxy-Authorization header should have been created with "Basic" keyword with a capital "B"

Steps to test

[na]

How has the user experience changed?

[na]

PR Tasks

[na]

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@Buluc-Celik-Ozbul Buluc-Celik-Ozbul requested a review from a team as a code owner June 23, 2022 07:46
@Buluc-Celik-Ozbul Buluc-Celik-Ozbul requested review from emilyrohrbough and removed request for a team June 23, 2022 07:46
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 23, 2022

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Jun 23, 2022

CLA assistant check
All committers have signed the CLA.

@Buluc-Celik-Ozbul Buluc-Celik-Ozbul changed the title Fixed Proxy-Authorization: Basic Fix: Proxy-Authorization: Basic Jun 23, 2022
@Buluc-Celik-Ozbul Buluc-Celik-Ozbul changed the title Fix: Proxy-Authorization: Basic fix: Proxy-Authorization: Basic Jun 23, 2022
@emilyrohrbough
Copy link
Member

@Buluc-Celik Thank you for the contribution! Can you please sign the CLA Agreement so we can move forward with this change?

@emilyrohrbough emilyrohrbough changed the title fix: Proxy-Authorization: Basic fix: Proxy-Authorization type Basic Jun 23, 2022
@Buluc-Celik-Ozbul
Copy link
Contributor Author

hi @emilyrohrbough I have signed it, I think it needs a bit of time to reflect though

@emilyrohrbough
Copy link
Member

@Buluc-Celik are you toggling between two git accounts?

@Buluc-Celik
Copy link
Contributor

Sorry about that, signed with both now

@emilyrohrbough
Copy link
Member

@Buluc-Celik no worries 😄

@lmiller1990
Copy link
Contributor

Looks good. We've got some flake blocking merging, but I can take it from here - it's not related to your change. Thanks!

@lmiller1990 lmiller1990 self-assigned this Jun 24, 2022
@Buluc-Celik
Copy link
Contributor

Thank you!

@Buluc-Celik
Copy link
Contributor

Hi, do you mind re-running the failed flaky test a few times to get this through, please? I would really like to get this in the next version and start using it.

@lmiller1990
Copy link
Contributor

Sure, I think the flake is fixed now. I will re-run.

@cypress
Copy link

cypress bot commented Jun 28, 2022



Test summary

37681 0 456 0Flakiness 11


Run details

Project cypress
Status Passed
Commit 73c91e5
Started Jun 29, 2022 5:35 AM
Ended Jun 29, 2022 5:54 AM
Duration 18:51 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

actions/click.cy.js Flakiness
1 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
2 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
xhr.cy.js Flakiness
1 ... > logs request + response headers
2 ... > logs Method, Status, URL, and XHR
3 ... > logs response
This comment includes only the first 5 flaky tests. See all 11 flaky tests in the Cypress Dashboard.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@lmiller1990
Copy link
Contributor

Less flake but still something isn't passing - figuring it out, thanks for waiting.

@emilyrohrbough emilyrohrbough changed the title fix: Proxy-Authorization type Basic fix: proxy-authorization type Basic Jun 28, 2022
@lmiller1990
Copy link
Contributor

@Buluc-Celik-Ozbul] please wait a moment - I am going to get this merged. If you keep back merging, it re runs everything. I can run "only from failed" which is more likely to succeed. I can take it from here.

@lmiller1990 lmiller1990 changed the title fix: proxy-authorization type Basic fix: proxy authorization type Basic Jun 29, 2022
@lmiller1990 lmiller1990 merged commit b1a51f9 into cypress-io:develop Jun 29, 2022
@Buluc-Celik-Ozbul
Copy link
Contributor Author

Sure, thank you

@lmiller1990
Copy link
Contributor

Thanks for your patience @Buluc-Celik-Ozbul - this will be in the next release. Those are usually every two weeks at maximum.

@Buluc-Celik-Ozbul
Copy link
Contributor Author

Thanks again!

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.

Proxy-Authorization header created with basic with a small "b"
6 participants