Minor XFCC refactoring

grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XForwardedClientCert.java

@@ -14,11 +14,20 @@
 import java.util.Collections;
 import java.util.List;
 
+import static com.salesforce.grpc.contrib.xfcc.XfccQuoteUtil.*;
+
 /**
  * x-forwarded-client-cert (XFCC) is a proxy header which indicates certificate information of part or all of the
  * clients or proxies that a request has flowed through, on its way from the client to the server.
  */
 public final class XForwardedClientCert {
+    static final String XFCC_BY = "By";
+    static final String XFCC_HASH = "Hash";
+    static final String XFCC_SAN = "SAN";
+    static final String XFCC_URI = "URI";
+    static final String XFCC_DNS = "DNS";
+    static final String XFCC_SUBJECT = "Subject";
+
     /**
      * The metadata key used to access any present {@link XForwardedClientCert} objects.
      */
@@ -89,33 +98,27 @@ public String getSubject() {
     public String toString() {
         List<String> kvp = new ArrayList<>();
         if (!by.isEmpty()) {
-            kvp.add("By=" + enquote(by));
+            kvp.add(toKvp(XFCC_BY, enquote(by)));
         }
         if (!hash.isEmpty()) {
-            kvp.add("Hash=" + enquote(hash));
+            kvp.add(toKvp(XFCC_HASH, enquote(hash)));
         }
         if (!sanUri.isEmpty()) {
-            kvp.add("URI=" + enquote(sanUri));
+            kvp.add(toKvp(XFCC_URI, enquote(sanUri)));
         }
-        for (String dns : sanDns) {
-            kvp.add("DNS=" + enquote(dns));
+        if (!sanDns.isEmpty()) {
+            for (String dns : sanDns) {
+                kvp.add(toKvp(XFCC_DNS, enquote(dns)));
+            }
         }
         if (!subject.isEmpty()) {
-            kvp.add("Subject=" + enquote(subject));
+            kvp.add(toKvp(XFCC_SUBJECT, enquote(subject)));
         }
 
         return String.join(";", kvp);
     }
 
-    private String enquote(String value) {
-        // Escape inner quotes with \"
-        value = value.replace("\"", "\\\"");
-
-        // Wrap in quotes if ,;= is present
-        if (value.contains(",") || value.contains(";") || value.contains("=")) {
-            value = "\"" + value + "\"";
-        }
-
-        return value;
+    private String toKvp(String key, String value) {
+        return key + "=" + value;
     }
 }

grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XfccParser.java

@@ -10,6 +10,9 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static com.salesforce.grpc.contrib.xfcc.XfccQuoteUtil.*;
+import static com.salesforce.grpc.contrib.xfcc.XForwardedClientCert.*;
+
 /**
  * {@code XfccParser} parses the {@code x-forwarded-client-cert} (XFCC) header populated by TLS-terminating
  * reverse proxies.
@@ -27,93 +30,32 @@ static List<XForwardedClientCert> parse(String header) {
 
         for (String element : quoteAwareSplit(header, ',')) {
             XForwardedClientCert cert = new XForwardedClientCert();
-            List<String> kvps = quoteAwareSplit(element, ';');
-            for (String kvp : kvps) {
-                List<String> l = quoteAwareSplit(kvp, '=');
-
-                if (l.get(0).toLowerCase().equals("by")) {
-                    cert.setBy(dequote(l.get(1)));
+            List<String> substrings = quoteAwareSplit(element, ';');
+            for (String substring : substrings) {
+                List<String> kvp = quoteAwareSplit(substring, '=');
+                String key = kvp.get(0).toLowerCase();
+                String value = kvp.get(1);
+
+                if (key.equalsIgnoreCase(XFCC_BY)) {
+                    cert.setBy(dequote(value));
                 }
-                if (l.get(0).toLowerCase().equals("hash")) {
-                    cert.setHash(dequote(l.get(1)));
+                if (key.equalsIgnoreCase(XFCC_HASH)) {
+                    cert.setHash(dequote(value));
                 }
                 // Use "SAN:" instead of "URI:" for backward compatibility with previous mesh proxy releases.
-                if (l.get(0).toLowerCase().equals("san") || l.get(0).toLowerCase().equals("uri")) {
-                    cert.setSanUri(dequote(l.get(1)));
+                if (key.equalsIgnoreCase(XFCC_SAN) || key.equalsIgnoreCase(XFCC_URI)) {
+                    cert.setSanUri(dequote(value));
                 }
-                if (l.get(0).toLowerCase().equals("dns")) {
-                    cert.addSanDns(dequote(l.get(1)));
+                if (key.equalsIgnoreCase(XFCC_DNS)) {
+                    cert.addSanDns(dequote(value));
                 }
-                if (l.get(0).toLowerCase().equals("subject")) {
-                    cert.setSubject(dequote(l.get(1)));
+                if (key.equalsIgnoreCase(XFCC_SUBJECT)) {
+                    cert.setSubject(dequote(value));
                 }
             }
             certs.add(cert);
         }
 
         return certs;
     }
-
-    // Break str into individual elements, splitting on delim (not in quotes)
-    private static List<String> quoteAwareSplit(String str, char delim) {
-        boolean inQuotes = false;
-        boolean inEscape = false;
-
-        List<String> elements = new ArrayList<>();
-        StringBuilder buffer = new StringBuilder();
-        for (char c : str.toCharArray()) {
-            if (c == delim && !inQuotes) {
-                elements.add(buffer.toString());
-                buffer = new StringBuilder();
-                inEscape = false;
-                continue;
-            }
-
-            if (c == '"') {
-                if (inQuotes) {
-                    if (!inEscape) {
-                        inQuotes = false;
-                    }
-                } else {
-                    inQuotes = true;
-
-                }
-                inEscape = false;
-                buffer.append(c);
-                continue;
-            }
-
-            if (c == '\\') {
-                if (!inEscape) {
-                    inEscape = true;
-                    buffer.append(c);
-                    continue;
-                }
-            }
-
-            // all other characters
-            inEscape = false;
-            buffer.append(c);
-        }
-
-        if (inQuotes) {
-            throw new RuntimeException("Quoted string not closed");
-        }
-
-        elements.add(buffer.toString());
-
-        return elements;
-    }
-
-    // Remove leading and tailing unescaped quotes, remove escaping from escaped internal quotes
-    private static String dequote(String str) {
-        str = str.replace("\\\"", "\"");
-        if (str.startsWith("\"")) {
-            str = str.substring(1);
-        }
-        if (str.endsWith("\"")) {
-            str = str.substring(0, str.length() - 1);
-        }
-        return str;
-    }
 }

grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XfccQuoteUtil.java

@@ -0,0 +1,99 @@
+/*
+ *  Copyright (c) 2017, salesforce.com, inc.
+ *  All rights reserved.
+ *  Licensed under the BSD 3-Clause license.
+ *  For full license text, see LICENSE.txt file in the repo root  or https://opensource.org/licenses/BSD-3-Clause
+ */
+package com.salesforce.grpc.contrib.xfcc;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Utility methods for quote escaping in XFCC headers.
+ */
+final class XfccQuoteUtil {
+    private XfccQuoteUtil() { }
+
+    /**
+     * Break str into individual elements, splitting on delim (not in quotes).
+     */
+    static List<String> quoteAwareSplit(String str, char delim) {
+        boolean inQuotes = false;
+        boolean inEscape = false;
+
+        List<String> elements = new ArrayList<>();
+        StringBuilder buffer = new StringBuilder();
+        for (char c : str.toCharArray()) {
+            if (c == delim && !inQuotes) {
+                elements.add(buffer.toString());
+                buffer.setLength(0); // clear
+                inEscape = false;
+                continue;
+            }
+
+            if (c == '"') {
+                if (inQuotes) {
+                    if (!inEscape) {
+                        inQuotes = false;
+                    }
+                } else {
+                    inQuotes = true;
+
+                }
+                inEscape = false;
+                buffer.append(c);
+                continue;
+            }
+
+            if (c == '\\') {
+                if (!inEscape) {
+                    inEscape = true;
+                    buffer.append(c);
+                    continue;
+                }
+            }
+
+            // all other characters
+            inEscape = false;
+            buffer.append(c);
+        }
+
+        if (inQuotes) {
+            throw new RuntimeException("Quoted string not closed");
+        }
+
+        elements.add(buffer.toString());
+
+        return elements;
+    }
+
+    /**
+     * Add escaping around double quote characters; wrap with quotes if special characters are present.
+     */
+    static String enquote(String value) {
+        // Escape inner quotes with \"
+        value = value.replace("\"", "\\\"");
+
+        // Wrap in quotes if ,;= is present
+        if (value.contains(",") || value.contains(";") || value.contains("=")) {
+            value = "\"" + value + "\"";
+        }
+
+        return value;
+    }
+
+    /**
+     * Remove leading and tailing unescaped quotes; remove escaping from escaped internal quotes.
+     */
+    static String dequote(String str) {
+        str = str.replace("\\\"", "\"");
+        if (str.startsWith("\"")) {
+            str = str.substring(1);
+        }
+        if (str.endsWith("\"")) {
+            str = str.substring(0, str.length() - 1);
+        }
+        return str;
+    }
+}

grpc-contrib/src/test/java/com/salesforce/grpc/contrib/xfcc/XfccMarshallerTest.java

@@ -12,6 +12,7 @@
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 public class XfccMarshallerTest {
     @Test
@@ -80,7 +81,14 @@ public void roundTripSimpleTest() {
 
     @Test
     public void roundTripUriAndDnsTest() {
+        XfccMarshaller marshaller = new XfccMarshaller();
+
+        String header = "By=http://frontend.lyft.com;Hash=468ed33be74eee6556d90c0149c1309e9ba61d6425303443c0748a02dd8de688;URI=http://testclient.lyft.com;DNS=lyft.com;DNS=www.lyft.com;Subject=\"/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=Test Client\"";
+
+        List<XForwardedClientCert> certs = marshaller.parseAsciiString(header);
+        String serialized = marshaller.toAsciiString(certs);
 
+        assertThat(serialized).isEqualTo(header);
     }
 
     @Test
@@ -121,4 +129,11 @@ public void roundTripEscapedQuotedTest() {
 
         assertThat(serialized).isEqualTo(header);
     }
+
+    @Test
+    public void malformedHeaderThrows() {
+        XfccMarshaller marshaller = new XfccMarshaller();
+        String header = "KABOOM!";
+        assertThatThrownBy(() -> marshaller.parseAsciiString(header)).isInstanceOf(RuntimeException.class);
+    }
 }

grpc-contrib/src/test/java/com/salesforce/grpc/contrib/xfcc/XfccParserTest.java

@@ -107,4 +107,12 @@ public void mismatchedQuotesThrows() {
 
         assertThatThrownBy(() -> XfccParser.parse(header)).isInstanceOf(RuntimeException.class);
     }
+
+    @Test
+    public void quotedKeyThrows() {
+        String header = "\"By\"=http://frontend.lyft.com;Hash=468ed33be74eee6556d90c0149c1309e9ba61d6425303443c0748a02dd8de688;" +
+                "Subject=\"/C=US/ST=CA/L=\\\"San Francisco\"/OU=Lyft/CN=Test Client\";SAN=http://testclient.lyft.com";
+
+        assertThatThrownBy(() -> XfccParser.parse(header)).isInstanceOf(RuntimeException.class);
+    }
 }