-
Notifications
You must be signed in to change notification settings - Fork 695
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 for out of bound read in std::strtol while parsing HTTP requests #1201
Conversation
Is it possible to use more modern way? https://en.cppreference.com/w/cpp/utility/from_chars It requires C++17 |
Thanks for your contribution @alexprabhat99. The There are a number of advantages to using the newer I recommend refactoring your PR to use the @Tachi107, feel free to chime in. |
(Just commenting since I noticed I was referenced on the issue) |
I sincerely apologize for the delay. @kiplingw I have made the required changes and incorporated @dgreatwood's points. This PR is now ready for review. |
No worries @alexprabhat99. Thanks for contributing. I've left some comments above. |
@kiplingw I have made the changes as per your review. |
Thanks @alexprabhat99, but it looks like there are multiple failures in CI related to unit tests. I'm not sure if this is a result of your PR per se, but normally they don't all bail like that. Unfortunately I don't have time to go into a deep dive right now, but maybe @Tachi107 can take a look. |
Hi Alex -
Thanks for incorporating the prior feedback.
I can quickly point out what Kip is referring to. Of course, if this is
already obvious you can disregard my comments :-)
pistache runs a number of github workflows (aka tests) upon git push,
including the ones defined in .github/workflows/linux.yaml in the project.
You can see the results of those tests either by going to your PR (
#1201) and scrolling down to
where you see "Some checks were not successful", then click on "linux".
Alternatively, at the top level of pistache, you can click on the "Actions"
button, scroll down to and click on your PR, and click on the "linux" test.
Either way, once you've opened the "linux" Test, you should see the log of
what happened. Searching the log for "FAIL" usually gives a pretty good
clue what went wrong :-)
Hope this helps.
D.
…On Mon, Apr 22, 2024 at 3:01 PM Kip ***@***.***> wrote:
Thanks @alexprabhat99 <https://github.com/alexprabhat99>, but it looks
like there are multiple failures in CI related to unit tests. I'm not sure
if this is a result of your PR per se, but normally they don't all bail
like that. Unfortunately I don't have time to go into a deep dive right
now, but maybe @Tachi107 <https://github.com/Tachi107> can take a look.
—
Reply to this email directly, view it on GitHub
<#1201 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFMA252OPZI57FQ7P2JAW3Y6WCEXAVCNFSM6AAAAABGEUROI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAZDIOJTGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
NOTICE: This email and its attachments may contain privileged and
confidential information, only for the viewing and use of the intended
recipient. If you are not the intended recipient, you are hereby notified
that any disclosure, copying, distribution, acting upon, or use of the
information contained in this email and its attachments is strictly
prohibited and that this email and its attachments must be immediately
returned to the sender and deleted from your system. If you received this
email erroneously, please notify the sender immediately. Xage Security,
Inc. and its affiliates will never request personal information (e.g.,
passwords, Social Security numbers) via email. Report suspicious emails to
***@***.*** ***@***.***>
|
I also meant to mention that the debian:testing tests are broken anyway
right now. But the others, such as debian:stable, should work once you've
perfected your PR.
And of course you can run at least one form of the test suite prior to
pushing to github using:
meson test -C build
(where build is your meson build directory)
Good luck.
On Mon, Apr 22, 2024 at 9:46 PM Duncan Greatwood ***@***.***>
wrote:
… Hi Alex -
Thanks for incorporating the prior feedback.
I can quickly point out what Kip is referring to. Of course, if this is
already obvious you can disregard my comments :-)
pistache runs a number of github workflows (aka tests) upon git push,
including the ones defined in .github/workflows/linux.yaml in the project.
You can see the results of those tests either by going to your PR (
#1201) and scrolling down to
where you see "Some checks were not successful", then click on "linux".
Alternatively, at the top level of pistache, you can click on the "Actions"
button, scroll down to and click on your PR, and click on the "linux" test.
Either way, once you've opened the "linux" Test, you should see the log of
what happened. Searching the log for "FAIL" usually gives a pretty good
clue what went wrong :-)
Hope this helps.
D.
On Mon, Apr 22, 2024 at 3:01 PM Kip ***@***.***> wrote:
> Thanks @alexprabhat99 <https://github.com/alexprabhat99>, but it looks
> like there are multiple failures in CI related to unit tests. I'm not sure
> if this is a result of your PR per se, but normally they don't all bail
> like that. Unfortunately I don't have time to go into a deep dive right
> now, but maybe @Tachi107 <https://github.com/Tachi107> can take a look.
>
> —
> Reply to this email directly, view it on GitHub
> <#1201 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAFMA252OPZI57FQ7P2JAW3Y6WCEXAVCNFSM6AAAAABGEUROI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAZDIOJTGU>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
NOTICE: This email and its attachments may contain privileged and
confidential information, only for the viewing and use of the intended
recipient. If you are not the intended recipient, you are hereby notified
that any disclosure, copying, distribution, acting upon, or use of the
information contained in this email and its attachments is strictly
prohibited and that this email and its attachments must be immediately
returned to the sender and deleted from your system. If you received this
email erroneously, please notify the sender immediately. Xage Security,
Inc. and its affiliates will never request personal information (e.g.,
passwords, Social Security numbers) via email. Report suspicious emails to
***@***.*** ***@***.***>
|
Fixes: #1193