Summary

Add header normalization and fetching for hosting service HTTP support.

Review Request #11199 — Created Sept. 24, 2020 and submitted Oct. 30, 2020, 2:22 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Blocks

11200

Reviewers

Groups

reviewboard

People

Description

This change works to standardize how we store headers, both for requests
and responses, in the new hosting service HTTP implementation. We now
store all headers as lowercase, and normalize on lookup.

HostingServiceHTTPResponse now has a get_header() method, just like
HostingServiceHTTPRequest. This is now preferred over accessing
headers directly, and will handle normalization on the caller's
behalf. This is future-proof and avoids any further confusion around
headers.

Testing Done

Unit tests pass on Python 2.7 and 3.6 through 3.8.

Commits

Summary	ID
Add header normalization and fetching for hosting service HTTP support. We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.	cecbe12e21956ad0351290633edbbbbfd5b0f51a

Summary

Add header normalization and fetching for hosting service HTTP support.

We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.

cecbe12e21956ad0351290633edbbbbfd5b0f51a

Issues

Description	From	Last Updated
E501 line too long (82 > 79 characters)	reviewbot	Sept. 24, 2020, 1:08 a.m.
The first and third Authorization header tests are the same test. The Content-length header tests are the same too. Let …	hailan	Sept. 25, 2020, 1:05 a.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/service.py (Diff revision 1)
The issue has been resolved. Show all issues
```
E501 line too long (82 > 79 characters)
```

Change Summary:

Fixed a long line.

Commits:

	Summary	ID
	Add header normalization and fetching for hosting service HTTP support. We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.	891c07beb0d2d61db97e4ce184bbaa0f94f069cb
	Add header normalization and fetching for hosting service HTTP support. We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.	97757595f1dde8a8ee7f606c0350e74d2170bcb3

Diff:

Revision 2 (+168 -22)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/hostingsvcs/tests/test_client.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

```
Great work!
```

reviewboard/hostingsvcs/tests/test_client.py (Diff revision 2)

The issue has been resolved. Show all issues

The first and third Authorization header tests are the same test.
The Content-length header tests are the same too.
Let me know if this is done on purpose.

chipx86 Sept. 25, 2020, 1:04 a.m.

Oops, definitely not! They were supposed to be lowercased. I did something wrong for sure. Good catch!

chipx86 Sept. 25, 2020, 1:05 a.m.

That is, the 3rd one was supposed to be lowercase. The 2nd was supposed to be all caps.

Change Summary:

Fixed some bad variations on HTTP headers in the unit tests.

Commits:

	Summary	ID
	Add header normalization and fetching for hosting service HTTP support. We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.	97757595f1dde8a8ee7f606c0350e74d2170bcb3
	Add header normalization and fetching for hosting service HTTP support. We've had some annoyances with properly looking up headers from an HTTP response in hosting service implementations in a consistent manner across versions of Review Board, and across versions of Python. Work has been done to fix this for GitHub, where this was more immediately noticed. This change works to standardize how we store headers, both for requests and responses, in the new hosting service HTTP implementation. We now store all headers as lowercase, and normalize on lookup. `HostingServiceHTTPResponse` now has a `get_header()` method, just like `HostingServiceHTTPRequest`. This is now preferred over accessing `headers` directly, and will handle normalization on the caller's behalf. This is future-proof and avoids any further confusion around headers.	cecbe12e21956ad0351290633edbbbbfd5b0f51a

Diff:

Revision 3 (+168 -22)

Show changes

	reviewboard/hostingsvcs/service.py
	reviewboard/hostingsvcs/tests/test_client.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.0.x (0a3bde5)