search

miyagawa / Starman

Starman is a high-performance preforking Perl PSGI web server
http://search.cpan.org/dist/Starman
Other
287 stars 84 forks source link

Fix case where POST body is the single character "0". #133

Closed olsonanl closed 5 years ago

olsonanl commented 5 years ago

This patch fixes the case where a POST request is the single character "0" (zero). Stock starman will hang in that case trying to read from a stream which will contain no more data.

miyagawa commented 5 years ago

This looks good to me, any idea why the CI tests are failing?

olsonanl commented 5 years ago

Hm, no. I'll see if I can get the test environment going here and replicate the problem. I'm actually not sure if the defined test is completely correct; it might be safer to check for '' since there are places in the code where '' is assigned (but they may come after the initial initialization where this problem crops up).

miyagawa commented 5 years ago

I pulled your change locally to see if the failure is CI specific and got the same failure.

miyagawa commented 5 years ago

I think the issue is that it's only checking defined and inputbuf could be empty ("").

olsonanl commented 5 years ago

Right; I noticed that there are other cases in the existing code that has the defined check. I can go thru and make a PR using the empty check instead.

miyagawa commented 5 years ago

It would be nice if you can add a unit test to illustrate the original problem (where content body is "0").

olsonanl commented 5 years ago

OK - I'll try to do that as well.

olsonanl commented 5 years ago

OK, my latest code has a new test single_zero.t that works with the new code as do all the other tests. However in the failure case it just hangs; I’m not sure how you properly handle that in the test harness.

On May 20, 2019, at 2:05 PM, Tatsuhiko Miyagawa notifications@github.com<mailto:notifications@github.com> wrote:

It would be nice if you can add a unit test to illustrate the original problem (where content body is "0").

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/miyagawa/Starman/pull/133?email_source=notifications&email_token=AAQNOITOSYDYATXFWG57PE3PWLY7LA5CNFSM4HNXD7FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVZY6RY#issuecomment-494112583, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAQNOIWAYDZALD5ZK4CBZXTPWLY7LANCNFSM4HNXD7FA.

olsonanl commented 5 years ago

Like that?

miyagawa commented 5 years ago

thanks!