Relax the validation of Location header when redirecting

[trustin]

Reported by @ohadgur at https://discord.com/channels/1087271586832318494/1209914423494311948

Motivation:

If a client is configured to follow redirects with followRedirects(), the client will validate the value of Location header before sending a follow-up request to the given redirect location. RedirectingClient validates and resolves the target location using URI.resolve() which rejects poorly encoded Location header values such as:

• Location: /foo bar (space should be percent-encoded)
• Location: /?${} ($, { and } should be percent-encoded.)

Such strict validation might not be suitable in real world scenarios and we could
normalize them just like a client validates and normalizes the initial request target.

Modifications:

• RedirectingClient now uses RequestTarget.forClient() to parse and normalize the target location so it is more tolerant to poorly encoded Location header values.
• RedirectingClient now implements its own relative path resolution logic. See
  RedirectingClient.resolveLocation() and `resolveRelativeLocation() for the detail.
• Added host and port properties to RequestTarget.
• Added host property to ClientRequestContext.
• Moved DefaultRequestTarget.findAuthority() to ArmeriaHttpUtil to reuse it in RedirectingClient.
• Miscellaneous:
  • Fixed a potential bug where RoutingContext.newPath() creates a new RequestTarget whose path is null
  • Fixed a bug where ClientRequestContext.uri() returns a double-encoded URI
    (Special thanks to @ohadgur for reporting this bug and suggesting a fix.)
  • Improved RequestTarget.forClient() to remove the port number in an absolute
    request target when possible, so that http://a:80 is normalized into http://a.

Result:

• (Bug fix) An Armeria client is now more tolerant to poorly encoded Location header values when following redirects.
• (New feature) You can now get the host and port part separately from a RequestTarget
  using RequestTarget.host() and RequestTarget.port().
• (New feature) You can now get the host part from authority of the request URI using
  ClientRequestContext.host().
• (Improvement) RequestTarget.forClient() now removes a redundant port number from
  the specified URI for simpler request target comparison. For example, https://foo and
  https://foo:443 are considered equal.

[github-actions]

🔍 Build Scan® (commit: 68927bc)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/g4ih343uteh4u
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/aj5hlw7dj4ums
build-self-hosted-unsafe-jdk-19-snapshot-blockhound	✅	https://ge.armeria.dev/s/4sf6rzaxcrmqm
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/6c2ieb6dlytu2
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/w4pn7phhsljm6
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/egig3rjo6hiwg
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/xdi2fuyvbqxag
build-macos-12-jdk-19	❌ (failure)	https://ge.armeria.dev/s/ycebzegk72blm

[trustin]

To-dos:

• Make resolveLocation() to always generate the full URL to simplify the overall logic and remove buildFullUri().
  • Make RequestTarget remove the port part when the port number is the default value.
• Support //host/path format in Location

[codecov]

Codecov Report

Attention: Patch coverage is 88.59060% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 74.06%. Comparing base (75d5af1) to head (68927bc).
Report is 7 commits behind head on main.

Files	Patch %	Lines
...com/linecorp/armeria/client/RedirectingClient.java	88.31%	2 Missing and 7 partials ⚠️
...ecorp/armeria/internal/common/ArmeriaHttpUtil.java	66.66%	1 Missing and 2 partials ⚠️
...a/internal/client/DefaultClientRequestContext.java	90.90%	1 Missing and 1 partial ⚠️
...rp/armeria/client/ClientRequestContextWrapper.java	0.00%	1 Missing ⚠️
.../armeria/internal/common/DefaultRequestTarget.java	96.29%	0 Missing and 1 partial ⚠️
...va/com/linecorp/armeria/server/RoutingContext.java	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5477       +/-   ##
===========================================
+ Coverage        0   74.06%   +74.06%     
- Complexity      0    20822    +20822     
===========================================
  Files           0     1801     +1801     
  Lines           0    76656    +76656     
  Branches        0     9783     +9783     
===========================================
+ Hits            0    56777    +56777     
- Misses          0    15253    +15253     
- Partials        0     4626     +4626     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[minwoox]

Looks great! Thanks!

[ikhoon]

Looks solid! 👍

[jrhee17]

Changes make sense 👍 Thanks! 🙇 👍 🙇

[trustin]

Thanks for reviewing, folks!

[ohadgur]

@trustin not urgent, of course, but when is this change expected to be released?

[jrhee17]

We're expecting to release 1.28.0 tomorrow