Fixes httpclient proxies. Fixes #6685 (#6694)

* Fix for #6685

* Fixed test breakage - SSL call wasn't within when defined(ssl) check
This commit is contained in:
Ganesh Viswanathan 2017-11-07 07:51:09 -06:00 • committed by Dominik Picheta
commit 686a2aaefa

View file

@ -469,7 +469,7 @@ proc request*(url: string, httpMethod: string, extraHeaders = "",
## **Deprecated since version 0.15.0**: use ``HttpClient.request`` instead. ## **Deprecated since version 0.15.0**: use ``HttpClient.request`` instead.
var r = if proxy == nil: parseUri(url) else: proxy.url var r = if proxy == nil: parseUri(url) else: proxy.url
var hostUrl = if proxy == nil: r else: parseUri(url) var hostUrl = if proxy == nil: r else: parseUri(url)
var headers = httpMethod.toUpper() var headers = httpMethod.toUpperAscii()
# TODO: Use generateHeaders further down once it supports proxies. # TODO: Use generateHeaders further down once it supports proxies.
var s = newSocket() var s = newSocket()
@ -713,10 +713,10 @@ proc downloadFile*(url: string, outputFilename: string,
proc generateHeaders(requestUrl: Uri, httpMethod: string, proc generateHeaders(requestUrl: Uri, httpMethod: string,
headers: HttpHeaders, body: string, proxy: Proxy): string = headers: HttpHeaders, body: string, proxy: Proxy): string =
# GET # GET
result = httpMethod.toUpper() result = httpMethod.toUpperAscii()
result.add ' ' result.add ' '
if proxy.isNil: if proxy.isNil or (not proxy.isNil and requestUrl.scheme == "https"):
# /path?query # /path?query
if requestUrl.path[0] != '/': result.add '/' if requestUrl.path[0] != '/': result.add '/'
result.add(requestUrl.path) result.add(requestUrl.path)
@ -1048,7 +1048,11 @@ proc newConnection(client: HttpClient | AsyncHttpClient,
client.currentURL.scheme != url.scheme or client.currentURL.scheme != url.scheme or
client.currentURL.port != url.port or client.currentURL.port != url.port or
(not client.connected): (not client.connected):
let isSsl = url.scheme.toLowerAscii() == "https" # Connect to proxy if specified
let connectionUrl =
if client.proxy.isNil: url else: client.proxy.url
let isSsl = connectionUrl.scheme.toLowerAscii() == "https"
if isSsl and not defined(ssl): if isSsl and not defined(ssl):
raise newException(HttpRequestError, raise newException(HttpRequestError,
@ -1056,31 +1060,55 @@ proc newConnection(client: HttpClient | AsyncHttpClient,
if client.connected: if client.connected:
client.close() client.close()
client.connected = false
# TODO: I should be able to write 'net.Port' here... # TODO: I should be able to write 'net.Port' here...
let port = let port =
if url.port == "": if connectionUrl.port == "":
if isSsl: if isSsl:
nativesockets.Port(443) nativesockets.Port(443)
else: else:
nativesockets.Port(80) nativesockets.Port(80)
else: nativesockets.Port(url.port.parseInt) else: nativesockets.Port(connectionUrl.port.parseInt)
when client is HttpClient: when client is HttpClient:
client.socket = await net.dial(url.hostname, port) client.socket = await net.dial(connectionUrl.hostname, port)
elif client is AsyncHttpClient: elif client is AsyncHttpClient:
client.socket = await asyncnet.dial(url.hostname, port) client.socket = await asyncnet.dial(connectionUrl.hostname, port)
else: {.fatal: "Unsupported client type".} else: {.fatal: "Unsupported client type".}
when defined(ssl): when defined(ssl):
if isSsl: if isSsl:
try: try:
client.sslContext.wrapConnectedSocket( client.sslContext.wrapConnectedSocket(
client.socket, handshakeAsClient, url.hostname) client.socket, handshakeAsClient, connectionUrl.hostname)
except: except:
client.socket.close() client.socket.close()
raise getCurrentException() raise getCurrentException()
# If need to CONNECT through proxy
if url.scheme == "https" and not client.proxy.isNil:
when defined(ssl):
# Pass only host:port for CONNECT
var connectUrl = initUri()
connectUrl.hostname = url.hostname
connectUrl.port = if url.port != "": url.port else: "443"
let proxyHeaderString = generateHeaders(connectUrl, $HttpConnect, newHttpHeaders(), "", client.proxy)
await client.socket.send(proxyHeaderString)
let proxyResp = await parseResponse(client, false)
if not proxyResp.status.startsWith("200"):
raise newException(HttpRequestError,
"The proxy server rejected a CONNECT request, " &
"so a secure connection could not be established.")
client.sslContext.wrapConnectedSocket(
client.socket, handshakeAsClient, url.hostname)
else:
raise newException(HttpRequestError,
"SSL support is not available. Cannot connect over SSL.")
# May be connected through proxy but remember actual URL being accessed
client.currentURL = url client.currentURL = url
client.connected = true client.connected = true
@ -1100,32 +1128,9 @@ proc requestAux(client: HttpClient | AsyncHttpClient, url: string,
headers: HttpHeaders = nil): Future[Response | AsyncResponse] headers: HttpHeaders = nil): Future[Response | AsyncResponse]
{.multisync.} = {.multisync.} =
# Helper that actually makes the request. Does not handle redirects. # Helper that actually makes the request. Does not handle redirects.
let connectionUrl =
if client.proxy.isNil: parseUri(url) else: client.proxy.url
let requestUrl = parseUri(url) let requestUrl = parseUri(url)
let savedProxy = client.proxy # client's proxy may be overwritten. await newConnection(client, requestUrl)
if requestUrl.scheme == "https" and not client.proxy.isNil:
when defined(ssl):
client.proxy.url = connectionUrl
var connectUrl = requestUrl
connectUrl.scheme = "http"
connectUrl.port = "443"
let proxyResp = await requestAux(client, $connectUrl, $HttpConnect)
if not proxyResp.status.startsWith("200"):
raise newException(HttpRequestError,
"The proxy server rejected a CONNECT request, " &
"so a secure connection could not be established.")
client.sslContext.wrapConnectedSocket(
client.socket, handshakeAsClient, requestUrl.hostname)
client.proxy = nil
else:
raise newException(HttpRequestError,
"SSL support not available. Cannot connect to https site over proxy.")
else:
await newConnection(client, connectionUrl)
let effectiveHeaders = client.headers.override(headers) let effectiveHeaders = client.headers.override(headers)
@ -1143,9 +1148,6 @@ proc requestAux(client: HttpClient | AsyncHttpClient, url: string,
client.getBody client.getBody
result = await parseResponse(client, getBody) result = await parseResponse(client, getBody)
# Restore the clients proxy in case it was overwritten.
client.proxy = savedProxy
proc request*(client: HttpClient | AsyncHttpClient, url: string, proc request*(client: HttpClient | AsyncHttpClient, url: string,
httpMethod: string, body = "", httpMethod: string, body = "",
headers: HttpHeaders = nil): Future[Response | AsyncResponse] headers: HttpHeaders = nil): Future[Response | AsyncResponse]