Skip to content

Commit

Permalink
More efficient and correct H1C-to-H2C upgrade
Browse files Browse the repository at this point in the history
Motivation:

The current HTTP/1-to-2 upgrade has the following slightly inter-related
issues:

1. When a user specifies an explicit session protocol (H1C/H1/H2C/H2)
instead of HTTP/HTTPS, the session creation must fail when the
negotiated protocol does not match exactly. For example, when a user
specified H2C, a rejected upgrade request should fail session creation.
2. When connecting to a host whose name is registered in /etc/hosts, the
'Host' header of the HEAD upgrade request is not set to the host name
but the host address.
3. When a user specifies HTTP/HTTPS as session protocol and the remote
host does not support H2C/H2, the client currently keeps sending the
HEAD upgrade request. We could cache the list of the hosts with no
HTTP/2 support, so we do not send upgrade requests unnecessarily.
4. Some servers send 'Connection: close' header when rejecting a upgrade
request, resulting disconnection. When this happens and a user specified
HTTP as session protocol, the client should retry the connection attempt
silently, so that a user does not notice the upgrade failure, because
it's not really a failure.

Modifications:

- (2) Fork DefaultHostsFileEntryResolver and HostsFileParser so that
  they do not omit the host name in the resolved InetAddress, where
  the host name is picked up from by Armeria
- (1) Add SessionProtocolNegotiationException
  - Mark the session creation as failure when protocol upgrade fails or
    the protocol is different from what user expected
    - See HttpConfigurator and HttpSessionChannelFactory
- (3) Add SessionProtocolNegotiationCache
  - Update the cache when protocol upgrade is rejected by remote host in
    HttpConfigurator
- (4) HttpSessionChannelFactory now retries the connection attempt when
  HttpConfigurator finds the upgrade response contains 'Connection:
  close' header.
- Add the test cases for (1, 3, 4) to ThriftOverHttpClientTServletIntegrationTest
- Miscellaneous:
  - When HTTP codec fails to decode a response, use the cause of the
    decoder failure as the invocation result, which is much more
    informative.
  - Add another variant of Exceptions.logIfUnexpected()
  - Add Exceptions.clearTrace() to remove the stack trace of an
    exception easily
  - Fix a bug where HttpServerHandler sends content in a response to a
    HEAD request. A response content of a HEAD request must be empty.

Result:

- Invocation now fails when the negotiated protocol does not match the
  desired protocol.
- The 'Host' header in a upgrade request now contains host name even if
  the host address was resolved via the /etc/hosts file
- HTTP/1-to-2 upgrade request is sent only when necessary.
- Http/1-to-2 upgrade deals better with 'Connection: close' headers.
  • Loading branch information
trustin committed Feb 5, 2016
1 parent dd37972 commit a822681
Show file tree
Hide file tree
Showing 15 changed files with 1,011 additions and 126 deletions.
@@ -0,0 +1,34 @@
/*
* Copyright 2015 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.client;

import java.net.InetAddress;
import java.util.Map;

import io.netty.resolver.HostsFileEntriesResolver;

/**
* Default {@link HostsFileEntriesResolver} that resolves hosts file entries only once.
*/
final class DefaultHostsFileEntriesResolver implements HostsFileEntriesResolver {
// TODO(trustin): Remove this fork once Netty 4.1.0.CR2 is out.
private final Map<String, InetAddress> entries = HostsFileParser.parseSilently();

@Override
public InetAddress address(String inetHost) {
return entries.get(inetHost);
}
}
175 changes: 175 additions & 0 deletions src/main/java/com/linecorp/armeria/client/HostsFileParser.java
@@ -0,0 +1,175 @@
/*
* Copyright 2015 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.client;

import static io.netty.util.internal.ObjectUtil.checkNotNull;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.Reader;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

import io.netty.util.NetUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

/**
* A parser for hosts files.
*/
final class HostsFileParser {
// TODO(trustin): Remove this fork once Netty 4.1.0.CR2 is out.
private static final String WINDOWS_DEFAULT_SYSTEM_ROOT = "C:\\Windows";
private static final String WINDOWS_HOSTS_FILE_RELATIVE_PATH = "\\system32\\drivers\\etc\\hosts";
private static final String X_PLATFORMS_HOSTS_FILE_PATH = "/etc/hosts";

private static final Pattern WHITESPACES = Pattern.compile("[ \t]+");

private static final InternalLogger logger = InternalLoggerFactory.getInstance(HostsFileParser.class);

private static File locateHostsFile() {
File hostsFile;
if (PlatformDependent.isWindows()) {
hostsFile = new File(System.getenv("SystemRoot") + WINDOWS_HOSTS_FILE_RELATIVE_PATH);
if (!hostsFile.exists()) {
hostsFile = new File(WINDOWS_DEFAULT_SYSTEM_ROOT + WINDOWS_HOSTS_FILE_RELATIVE_PATH);
}
} else {
hostsFile = new File(X_PLATFORMS_HOSTS_FILE_PATH);
}
return hostsFile;
}

/**
* Parse hosts file at standard OS location.
*
* @return a map of hostname or alias to {@link InetAddress}
*/
static Map<String, InetAddress> parseSilently() {
File hostsFile = locateHostsFile();
try {
return parse(hostsFile);
} catch (IOException e) {
logger.warn("Failed to load and parse hosts file at " + hostsFile.getPath(), e);
return Collections.emptyMap();
}
}

/**
* Parse hosts file at standard OS location.
*
* @return a map of hostname or alias to {@link InetAddress}
* @throws IOException file could not be read
*/
static Map<String, InetAddress> parse() throws IOException {
return parse(locateHostsFile());
}

/**
* Parse a hosts file.
*
* @param file the file to be parsed
* @return a map of hostname or alias to {@link InetAddress}
* @throws IOException file could not be read
*/
static Map<String, InetAddress> parse(File file) throws IOException {
checkNotNull(file, "file");
if (file.exists() && file.isFile()) {
return parse(new BufferedReader(new FileReader(file)));
} else {
return Collections.emptyMap();
}
}

/**
* Parse a reader of hosts file format.
*
* @param reader the file to be parsed
* @return a map of hostname or alias to {@link InetAddress}
* @throws IOException file could not be read
*/
static Map<String, InetAddress> parse(Reader reader) throws IOException {
checkNotNull(reader, "reader");
BufferedReader buff = new BufferedReader(reader);
try {
Map<String, InetAddress> entries = new HashMap<>();
String line;
while ((line = buff.readLine()) != null) {
// remove comment
int commentPosition = line.indexOf('#');
if (commentPosition != -1) {
line = line.substring(0, commentPosition);
}
// skip empty lines
line = line.trim();
if (line.isEmpty()) {
continue;
}

// split
List<String> lineParts = new ArrayList<>();
for (String s: WHITESPACES.split(line)) {
if (!s.isEmpty()) {
lineParts.add(s);
}
}

// a valid line should be [IP, hostname, alias*]
if (lineParts.size() < 2) {
// skip invalid line
continue;
}

byte[] ipBytes = NetUtil.createByteArrayFromIpAddressString(lineParts.get(0));

if (ipBytes == null) {
// skip invalid IP
continue;
}

// loop over hostname and aliases
for (int i = 1; i < lineParts.size(); i ++) {
String hostname = lineParts.get(i);
if (!entries.containsKey(hostname)) {
// trying to map a host to multiple IPs is wrong
// only the first entry is honored
entries.put(hostname, InetAddress.getByAddress(hostname, ipBytes));
}
}
}
return entries;
} finally {
try {
buff.close();
} catch (IOException e) {
logger.warn("Failed to close a reader", e);
}
}
}

/**
* Can't be instantiated.
*/
private HostsFileParser() {}
}

0 comments on commit a822681

Please sign in to comment.