Page MenuHomePhabricator

Allow empty Reason-Phrases
ClosedPublic

Authored by nfirmani on Aug 31 2018, 11:24 PM.
Tags
None
Referenced Files
F19582534: D19628.diff
Sun, Feb 1, 10:40 PM
F19507786: D19628.diff
Fri, Jan 9, 11:09 PM
F18897301: D19628.id46918.diff
Nov 7 2025, 5:02 PM
F18872904: D19628.id.diff
Nov 5 2025, 6:51 AM
F18869206: D19628.diff
Nov 4 2025, 10:24 AM
F18849113: D19628.id46921.diff
Oct 30 2025, 7:49 AM
F18847189: D19628.id46918.diff
Oct 29 2025, 11:15 PM
F18846389: D19628.diff
Oct 29 2025, 6:44 PM
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
Branch
nf_HTTP_200_is_my_favorite_status_code_thanks_JIRA_for_this_magic_🧙_
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20724
Build 28170: arc lint + arc unit

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.