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 validateHttpVersion to be RFC 1945 and RFC 7230 compliant #4368

Merged
merged 3 commits into from
Nov 24, 2024

Conversation

leissbua
Copy link
Contributor

@leissbua leissbua commented Nov 18, 2024

Description (*)

Mage_HTTP_Client_Curl is expecting the HTTP Response Header to have 3 elements.

  1. HTTP-Version
  2. Status-Code
  3. Reason-Phrase

However, the Reason-Phrase is optional, e.g. Apple Pay does not return OK Reason-Phrase on the current validationURL()endpoint which is HTTP 1.1. I fixed the code to allow 2 or 3 elements as it does for HTTP 2.0 in the current version

@github-actions github-actions bot added Component: lib/Mage Relates to lib/Mage Component: lib/* Relates to lib/* labels Nov 18, 2024
Copy link
Contributor

@sreichel sreichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@leissbua
Copy link
Contributor Author

What does "." mean @sreichel

@sreichel
Copy link
Contributor

@leissbua was a comment on your first commit. The change you have reverted,

Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref https://www.rfc-editor.org/rfc/rfc7230#section-3.1.2:

The reason-phrase element exists for the sole purpose of providing a textual description associated with the numeric status code, mostly out of deference to earlier Internet application protocols that were more frequently used with interactive text clients. A client SHOULD ignore the reason-phrase content.

@sreichel
Copy link
Contributor

Thanks for that info.

@sreichel sreichel merged commit 41ea3c6 into OpenMage:main Nov 24, 2024
19 checks passed
@sreichel
Copy link
Contributor

fballiano added a commit to MahoCommerce/maho that referenced this pull request Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: lib/Mage Relates to lib/Mage Component: lib/* Relates to lib/*
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants