Skip to content

Conversation

@fhammerl
Copy link
Contributor

@fhammerl fhammerl commented Nov 30, 2022

Closes #2165
Uses behaviour from UserAgentUtility.cs?

Bug

Linux Liquorix has an OS description that contains mismatched brackets, so the constructor of ProductInfoHeaderValue throws.

Solution

Drop all () to prevent nested UserAgent comments
I don't know of other chars that would need to be sanitized.

Context

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent

We use dotnet's OSDescription to get a string that we then include in our UserAgent header as a ProductInfoHeaderValue.

The constructor of ProductInfoHeaderValue validates if a string is compliant with UserAgent formatting. There aren't many requirements, but one of them is matching brackets -> ()
Brackets represent a comment in a UserAgent.

@fhammerl fhammerl requested a review from a team as a code owner November 30, 2022 13:13
@fhammerl fhammerl changed the title Sanitize OS Desc for UserAgents Drop '(' and ')' from OS.Description so OS.Description doesn't fail User-Agent validation when it contains mismatched brackets Nov 30, 2022
nikola-jokic
nikola-jokic previously approved these changes Nov 30, 2022
Copy link
Contributor

@nikola-jokic nikola-jokic left a comment

Choose a reason for hiding this comment

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

LGTM

AvaStancu
AvaStancu previously approved these changes Nov 30, 2022
@fhammerl fhammerl dismissed stale reviews from AvaStancu and nikola-jokic via 74440ea December 1, 2022 22:53
@fhammerl fhammerl changed the title Drop '(' and ')' from OS.Description so OS.Description doesn't fail User-Agent validation when it contains mismatched brackets Replace '(' and ')' with '[' and '] from OS.Description so OS.Description doesn't fail User-Agent validation Feb 8, 2023
@fhammerl fhammerl changed the title Replace '(' and ')' with '[' and '] from OS.Description so OS.Description doesn't fail User-Agent validation Replace '(' and ')' with '[' and '] from OS.Description so it doesn't fail User-Agent validation Feb 8, 2023
@fhammerl fhammerl changed the title Replace '(' and ')' with '[' and '] from OS.Description so it doesn't fail User-Agent validation Replace '(' and ')' with '[' and '] from OS.Description so it doesn't fail User-Agent header validation Feb 8, 2023
@fhammerl fhammerl merged commit 97195ba into main Feb 8, 2023
@fhammerl fhammerl deleted the fhammerl/sanitize-user-agent-string branch February 8, 2023 16:42
nikola-jokic pushed a commit to nikola-jokic/runner that referenced this pull request May 12, 2023
… fail User-Agent header validation (actions#2288)

* Sanitize OS Desc for UserAgents

* Only drop brackets if needed, refactoring

* Add missing ')'

* Readd missing brackets around '(header)'

* Add comments

* Use bracket solution from SDK

* Rename tests
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Aug 14, 2024
… fail User-Agent header validation (actions#2288)

* Sanitize OS Desc for UserAgents

* Only drop brackets if needed, refactoring

* Add missing ')'

* Readd missing brackets around '(header)'

* Add comments

* Use bracket solution from SDK

* Rename tests
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.

Liquorix kernel breaks self-hosted install

5 participants