Page MenuHomePhabricator

Allow empty Reason-Phrases
ClosedPublic

Authored by nfirmani on Aug 31 2018, 11:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 14, 10:32 PM
Unknown Object (File)
Tue, Dec 10, 12:49 AM
Unknown Object (File)
Tue, Nov 26, 1:58 AM
Unknown Object (File)
Nov 10 2024, 1:43 AM
Unknown Object (File)
Oct 23 2024, 11:46 AM
Unknown Object (File)
Oct 9 2024, 10:00 AM
Unknown Object (File)
Oct 1 2024, 8:57 PM
Unknown Object (File)
Sep 6 2024, 3:46 AM
Subscribers

Details

Summary

According to RFC 7230, the reason-phrase section of a HTTP response can be empty. This regex expects a space before an optional reason-phrase. This diff patches it so the space is optional as well.

I discovered recently when integrating a JIRA instance with a self hosted version of Phabricator that the JIRA OAUTH endpoint would respond with just HTTP/1.1 200 instead of the much more common HTTP/1.1 200 OK message. This caused this regex to fail to match the successful response and JIRA account linking to fail, disappointing my corporate overlords.

Test Plan

I tested this regex in isolation using a scratch PHP file to check its behavior. I then patched it into our instance of Phabricator and tried linking a JIRA account to a Phabricator account and saw that it succeeded.

An example response that would not parse correctly before is

HTTP/1.1 200

Content-Type: text/plain

content

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PhutilHTTPResponseParser and the actual test coverage on that should get this change too since parseRawHTTPResponse() is theoretically on the chopping block, but I can take care of that. Thanks!

This revision is now accepted and ready to land.Sep 4 2018, 3:22 PM
This revision was automatically updated to reflect the committed changes.