CIO engine no longer sends port in "Host" header

[royneely]

Fixes #1294

see #1294 for details

[royneely]

I looked through the tests before I submitted to see if this change would cause anything to fail. I did not see anything.

I would be happy to fix any broken tests, but I can't access the test failure results. Is there a way that I can access them?

[royneely]

Oh nevermind, I figured it out. sorry. looking at test results now.

[royneely]

fixed the two tests that were broken.

I have no idea why this one would be failing..

ktor/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/ContentTest.kt

Lines 86 to 95 in 2296583

	fun testTextContent() = clientTests(listOf("js")) {
	test { client ->
	testSize.forEach { size ->
	val content = makeString(size)
	val response = client.echo<String>(TextContent(content, ContentType.Text.Plain))
	assertArrayEquals("Test fail with size: $size", content.toByteArray(), response.toByteArray())
	}
	}
	}

ContentTest.testTextContent[Apache]

java.util.concurrent.CancellationException: Fail to execute request
	at    (Coroutine boundary. ( )
	at kotlinx.coroutines.io.ByteBufferChannel.readRemainingSuspend(ByteBufferChannel.kt:2201)
	at io.ktor.client.features.HttpPlainText$Feature$install$2.invokeSuspend(HttpPlainText.kt:140)
Caused by: java.util.concurrent.CancellationException: Fail to execute request
	at kotlinx.coroutines.ExceptionsKt.CancellationException(Exceptions.kt:33)
	at io.ktor.client.engine.apache.ApacheResponseConsumer.failed(ApacheResponseConsumer.kt:118)
	at org.apache.http.impl.nio.client.DefaultClientExchangeHandlerImpl.executionFailed(DefaultClientExchangeHandlerImpl.java:99)
	at org.apache.http.impl.nio.client.AbstractClientExchangeHandler.failed(AbstractClientExchangeHandler.java:426)
	at org.apache.http.nio.protocol.HttpAsyncRequestExecutor.timeout(HttpAsyncRequestExecutor.java:381)
	at org.apache.http.impl.nio.client.InternalIODispatch.onTimeout(InternalIODispatch.java:92)
	at org.apache.http.impl.nio.client.InternalIODispatch.onTimeout(InternalIODispatch.java:39)
	at org.apache.http.impl.nio.reactor.AbstractIODispatch.timeout(AbstractIODispatch.java:175)
	at org.apache.http.impl.nio.reactor.BaseIOReactor.sessionTimedOut(BaseIOReactor.java:263)
	at org.apache.http.impl.nio.reactor.AbstractIOReactor.timeoutCheck(AbstractIOReactor.java:492)
	at org.apache.http.impl.nio.reactor.BaseIOReactor.validate(BaseIOReactor.java:213)
	at org.apache.http.impl.nio.reactor.AbstractIOReactor.execute(AbstractIOReactor.java:280)
	at org.apache.http.impl.nio.reactor.BaseIOReactor.execute(BaseIOReactor.java:104)
	at org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor$Worker.run(AbstractMultiworkerIOReactor.java:588)
	at java.base/java.lang.Thread.run(Thread.java:844)
Caused by: java.net.SocketTimeoutException: 10,000 milliseconds timeout on connection http-outgoing-25 [ACTIVE]
	... 11 more

Flakey test maybe? not sure.

[Bluexin]

See ietf on the Host header :

Host = uri-host [ ":" port ] ;

Port is optional, but part of the spec.

It follows the http URI scheme, defined in section 2.7.1 :

If the port subcomponent is empty or not given, TCP port 80 (the reserved port for WWW services) is the default.

Maybe we can just have it not specify default ports ?

[royneely]

Maybe we can just have it not specify default ports ?

@Bluexin Good idea. that seems to align with this ietf spec for host headers

https://tools.ietf.org/html/rfc2616#section-14.23

A "host" without any trailing port information implies the default
port for the service requested (e.g., "80" for an HTTP URL). For
example, a request on the origin server for
http://www.w3.org/pub/WWW/ would properly include:

  GET /pub/WWW/ HTTP/1.1
  Host: www.w3.org

I updated this PR to reflect that policy.

[e5l]

Hi @Evyy, thanks for the PR. Merged in e5l/develop.