Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow incomplete url builder tostring #4357

Merged
merged 1 commit into from
Nov 3, 2018

Conversation

yschimke
Copy link
Collaborator

@yschimke yschimke commented Nov 2, 2018

toString is useful for debug even when incomplete

@yschimke yschimke changed the title allow incomplete url builder tostring Allow incomplete url builder tostring Nov 2, 2018
@yschimke
Copy link
Collaborator Author

yschimke commented Nov 3, 2018

n.b. the builder.toString is used for the HttpUrl.toString representation, but those fields are non null and > 0 for any valid HttpUrl.

@yschimke yschimke merged commit 59ab235 into square:master Nov 3, 2018
Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Good idea

@Test public void incompleteBuilderToString() {
assertEquals("https:///path",
new HttpUrl.Builder().scheme("https").encodedPath("/path").toString());
assertEquals("://host.com/path",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The : is part of the scheme, so it should be omitted unless the scheme is present.

result.append(scheme);
if (scheme != null) {
result.append(scheme);
}
result.append("://");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only append : if the scheme is present?

@yschimke
Copy link
Collaborator Author

yschimke commented Nov 3, 2018

#4361

SeniorZhai added a commit to SeniorZhai/okhttp that referenced this pull request Nov 15, 2018
* commit '1f7e796e6e658df34a98276b2092a81de118937d':
  Cleanup HttpLoggingInterceptor (square#4346)
  Add a LoggingEventListener and use it in okcurl (square#4353)
  Preemptive auth for proxy CONNECT
  Relax handling of Cache-Control: immutable
  Add some docs for Cache class (square#4375)
  Fix connection leaks on failed web socket upgrades.
  Don't specify a crypto provider in HeldCertificate.
  Confirm that call timeouts don't apply to SSE or web sockets.
  Add APIs to configure the client's default call timeout. (square#4369)
  Recover from executor shutdowns gracefully. (square#4365)
  Make the nested BasicAuthInterceptor static (square#4368)
  Add basic auth interceptor recipe (square#4336)
  Whole operation timeouts
  Make scheme names case-sensitive again.
  Remove colon when scheme missing in builder toString (square#4361)
  CipherSuite init speedup (square#4340)
  Limit the use of regexes in the RFC 7235 challenge parser.
  Allow incomplete url builder toString usage (square#4357)
  APIs to set date headers
  Upgrade Conscrypt to 1.4.0 (was 1.3.0)
@yschimke yschimke deleted the urlbuilder_tostring branch February 9, 2019 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants