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

Properly handle incorrect port numbers in parseURL (fixes #4200) #4213

Closed
wants to merge 2 commits into from
Closed

Properly handle incorrect port numbers in parseURL (fixes #4200) #4213

wants to merge 2 commits into from

Conversation

ckeshava
Copy link
Collaborator

This Pull Request fixes issue 4200 (#4200).

Previously, inputs for very large port numbers (more than the 16 bit capacity of the field) was handled incorrectly i.e. lexicalCast returns a zero for such inputs.

The code in this PR returns a boolean value of false if the port number is zero.
This code also handles the case of an input of zero (0) to the port number field.

@ckeshava ckeshava marked this pull request as draft June 17, 2022 21:43
@ckeshava ckeshava marked this pull request as ready for review June 17, 2022 21:44
@nbougalis nbougalis added the Bug label Jun 17, 2022
@ckeshava
Copy link
Collaborator Author

With regards to the failed Github Actions task, clang-format is not a recognised git command. Do I need to use an extension/tool (something like this? https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/git-clang-format) to format my code?

@scottschurr
Copy link
Collaborator

I think you can use homebrew to install $ brew install clang-format. I always just try to remember to invoke clang-format before I push to my GitHub repo. There are other folks who have done something or other that is more automated.

@ckeshava
Copy link
Collaborator Author

Okay, thanks for the suggestion Scott! I'll look into that.

@nbougalis nbougalis mentioned this pull request Jul 11, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants