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: Improve set_port validation #839

Merged
merged 18 commits into from
Jan 12, 2025

Conversation

mertcanaltin
Copy link
Contributor

@mertcanaltin mertcanaltin commented Jan 11, 2025

  • Added logic to remove non-digit characters from the trimmed string using std::find_if_not and erase.
  • Ensures only numeric values are processed as the port.
  • Fixes cases where inputs like "8080stuff" or "8080/path" would previously cause issues, now correctly handling them as "8080".

Example:

  • Input: "8080stuff"Processed as: "8080"
  • Input: "invalid80"Processed as: "" (invalid)

Fixes #826

src/url.cpp Outdated Show resolved Hide resolved
Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

I think we should remove the extra code right now and just focus on simplifying the code

src/url.cpp Outdated Show resolved Hide resolved
src/url.cpp Outdated Show resolved Hide resolved
src/url_aggregator.cpp Outdated Show resolved Hide resolved
src/url_aggregator.cpp Outdated Show resolved Hide resolved
tests/basic_tests.cpp Outdated Show resolved Hide resolved
mertcanaltin and others added 5 commits January 12, 2025 22:47
Co-authored-by: Yagiz Nizipli <[email protected]>
Co-authored-by: Yagiz Nizipli <[email protected]>
Co-authored-by: Yagiz Nizipli <[email protected]>
Co-authored-by: Yagiz Nizipli <[email protected]>
Co-authored-by: Yagiz Nizipli <[email protected]>
@mertcanaltin
Copy link
Contributor Author

mertcanaltin commented Jan 12, 2025

I think we should remove the extra code right now and just focus on simplifying the code

thank you very much for the suggestions, I have accepted improvements that remove excess code, they have been instructive for me

tests/basic_tests.cpp Outdated Show resolved Hide resolved
src/url_aggregator.cpp Outdated Show resolved Hide resolved
src/url_aggregator.cpp Show resolved Hide resolved
src/url.cpp Show resolved Hide resolved
src/url.cpp Outdated Show resolved Hide resolved
tests/basic_tests.cpp Outdated Show resolved Hide resolved
@anonrig anonrig merged commit 0394a7c into ada-url:main Jan 12, 2025
31 of 34 checks passed
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.

set_port returns true for invalid value
2 participants