Merge pull request #273 from typesafehub/404-url

Treat 404 from a URL as missing file, not fatal error

config/src/main/java/com/typesafe/config/impl/Parseable.java

@@ -14,6 +14,7 @@ import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
+import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -463,32 +464,46 @@ public abstract class Parseable implements ConfigParseable {
 
         @Override
         protected Reader reader(ConfigParseOptions options) throws IOException {
-            if (ConfigImpl.traceLoadsEnabled())
+            try {
-                trace("Loading config from a URL: " + input.toExternalForm());
-            URLConnection connection = input.openConnection();
-
-            // allow server to serve multiple types from one URL
-            String acceptContent = acceptContentType(options);
-            if (acceptContent != null) {
-                connection.setRequestProperty("Accept", acceptContent);
-            }
-
-            connection.connect();
-
-            // save content type for later
-            contentType = connection.getContentType();
-            if (contentType != null) {
                 if (ConfigImpl.traceLoadsEnabled())
-                    trace("URL sets Content-Type: '" + contentType + "'");
+                    trace("Loading config from a URL: " + input.toExternalForm());
-                contentType = contentType.trim();
+                URLConnection connection = input.openConnection();
-                int semi = contentType.indexOf(';');
+
-                if (semi >= 0)
+                // allow server to serve multiple types from one URL
-                    contentType = contentType.substring(0, semi);
+                String acceptContent = acceptContentType(options);
+                if (acceptContent != null) {
+                    connection.setRequestProperty("Accept", acceptContent);
+                }
+
+                connection.connect();
+
+                // save content type for later
+                contentType = connection.getContentType();
+                if (contentType != null) {
+                    if (ConfigImpl.traceLoadsEnabled())
+                        trace("URL sets Content-Type: '" + contentType + "'");
+                    contentType = contentType.trim();
+                    int semi = contentType.indexOf(';');
+                    if (semi >= 0)
+                        contentType = contentType.substring(0, semi);
+                }
+
+                InputStream stream = connection.getInputStream();
+
+                return readerFromStream(stream);
+            } catch (FileNotFoundException fnf) {
+                // If the resource is not found (HTTP response
+                // code 404 or something alike), then it's fine to
+                // treat it according to the allowMissing setting
+                // and "include" spec.  But if we have something
+                // like HTTP 503 it seems to be better to fail
+                // early, because this may be a sign of broken
+                // environment. Java throws FileNotFoundException
+                // if it sees 404 or 410.
+                throw fnf;
+            } catch (IOException e) {
+                throw new ConfigException.BugOrBroken("Cannot load config from URL: " + input.toExternalForm(), e);
             }
-
-            InputStream stream = connection.getInputStream();
-
-            return readerFromStream(stream);
         }
 
         @Override

config/src/test/scala/com/typesafe/config/impl/HttpTest.scala

@@ -0,0 +1,147 @@
+package com.typesafe.config.impl
+
+import org.junit.Assert._
+import org.junit._
+import org.junit.BeforeClass
+import java.net.URL
+import com.typesafe.config.ConfigFactory
+import com.typesafe.config.ConfigParseOptions
+import com.typesafe.config.ConfigSyntax
+import com.typesafe.config.ConfigException
+
+class HttpTest extends TestUtils {
+    import HttpTest._
+
+    private def foreachSyntax(body: Option[ConfigSyntax] => Unit): Unit = {
+        for (syntax <- Seq(Some(ConfigSyntax.JSON), Some(ConfigSyntax.CONF), Some(ConfigSyntax.PROPERTIES), None))
+            body(syntax)
+    }
+
+    private def foreachSyntaxOptions(body: ConfigParseOptions => Unit): Unit = foreachSyntax { syntaxOption =>
+        val options = syntaxOption map { syntax =>
+            ConfigParseOptions.defaults.setSyntax(syntax)
+        } getOrElse {
+            ConfigParseOptions.defaults()
+        }
+
+        body(options)
+    }
+
+    def url(path: String): URL = new URL(s"$baseUrl/$path")
+
+    @Test
+    def parseEmpty(): Unit = {
+        foreachSyntaxOptions { options =>
+            val conf = ConfigFactory.parseURL(url("empty"), options)
+            assertTrue("empty conf was parsed", conf.root.isEmpty)
+        }
+    }
+
+    @Test
+    def parseFooIs42(): Unit = {
+        foreachSyntaxOptions { options =>
+            val conf = ConfigFactory.parseURL(url("foo"), options)
+            assertEquals(42, conf.getInt("foo"))
+        }
+    }
+
+    @Test
+    def notFoundThrowsIO(): Unit = {
+        val e = intercept[ConfigException.IO] {
+            ConfigFactory.parseURL(url("notfound"), ConfigParseOptions.defaults().setAllowMissing(false))
+        }
+        assertTrue(s"expected different exception for notfound, got $e", e.getMessage.contains("/notfound"))
+    }
+
+    @Test
+    def internalErrorThrowsBroken(): Unit = {
+        val e = intercept[ConfigException.BugOrBroken] {
+            ConfigFactory.parseURL(url("error"), ConfigParseOptions.defaults().setAllowMissing(false))
+        }
+        assertTrue(s"expected different exception for error url, got $e", e.getMessage.contains("/error"))
+    }
+
+    @Test
+    def notFoundDoesNotThrowIfAllowingMissing(): Unit = {
+        val conf = ConfigFactory.parseURL(url("notfound"), ConfigParseOptions.defaults().setAllowMissing(true))
+        assertEquals(0, conf.root.size)
+    }
+
+    @Test
+    def internalErrorThrowsEvenIfAllowingMissing(): Unit = {
+        val e = intercept[ConfigException.BugOrBroken] {
+            ConfigFactory.parseURL(url("error"), ConfigParseOptions.defaults().setAllowMissing(true))
+        }
+        assertTrue(s"expected different exception for error url when allowing missing, got $e", e.getMessage.contains("/error"))
+    }
+
+    @Test
+    def relativeInclude(): Unit = {
+        val conf = ConfigFactory.parseURL(url("includes-a-friend"))
+        assertEquals(42, conf.getInt("foo"))
+        assertEquals(43, conf.getInt("bar"))
+    }
+}
+
+object HttpTest {
+    import ToyHttp.{ Request, Response }
+
+    final val jsonContentType = "application/json"
+    final val propertiesContentType = "text/x-java-properties"
+    final val hoconContentType = "application/hocon"
+
+    private var server: Option[ToyHttp] = None
+
+    def port = server.map(_.port).getOrElse(throw new Exception("http server isn't running"))
+    def baseUrl = s"http://127.0.0.1:$port"
+
+    private def handleThreeTypes(request: Request, json: String, props: String, hocon: String): Response = {
+        request.headers.get("accept") match {
+            case Some(`jsonContentType`) | None => Response(200, jsonContentType, json)
+            case Some(`propertiesContentType`) => Response(200, propertiesContentType, props)
+            case Some(`hoconContentType`) => Response(200, hoconContentType, hocon)
+            case Some(other) => Response(500, "text/plain", s"bad content type '$other'")
+        }
+    }
+
+    private def handleRequest(request: Request): Response = {
+        request.path match {
+            case "/empty" =>
+                handleThreeTypes(request, "{}", "", "")
+
+            case "/foo" | "/foo.conf" =>
+                handleThreeTypes(request, "{ \"foo\" : 42 }", "foo:42", "foo=42")
+
+            case "/notfound" =>
+                Response(404, "text/plain", "This is never found")
+
+            case "/error" =>
+                Response(500, "text/plain", "This is always an error")
+
+            case "/includes-a-friend" =>
+                // currently, a suffix-less include like this will cause
+                // us to search for foo.conf, foo.json, foo.properties, but
+                // not load plain foo.
+                Response(200, hoconContentType, """
+                  include "foo"
+                  include "foo/bar"
+                  """)
+
+            case "/foo/bar.conf" =>
+                Response(200, hoconContentType, "{ bar = 43 }")
+
+            case path =>
+                Response(404, "text/plain", s"Never heard of '$path'")
+        }
+    }
+
+    @BeforeClass
+    def startServer(): Unit = {
+        server = Some(ToyHttp(handleRequest))
+    }
+
+    @AfterClass
+    def stopServer(): Unit = {
+        server.foreach(_.stop())
+    }
+}

config/src/test/scala/com/typesafe/config/impl/ToyHttp.scala

@@ -0,0 +1,141 @@
+package com.typesafe.config.impl
+
+import java.net.ServerSocket
+import java.net.InetSocketAddress
+import scala.annotation.tailrec
+import scala.util.control.NonFatal
+import java.net.Socket
+import java.io.BufferedReader
+import java.io.IOException
+import java.io.EOFException
+import java.io.OutputStream
+import java.text.SimpleDateFormat
+import java.util.Locale
+import java.util.TimeZone
+import java.io.PrintWriter
+import java.io.OutputStreamWriter
+import java.nio.charset.StandardCharsets
+import java.util.Date
+
+// terrible http server that's good enough for our test suite
+final class ToyHttp(handler: ToyHttp.Request => ToyHttp.Response) {
+
+    import ToyHttp.{ Request, Response }
+
+    private final val serverSocket = new ServerSocket()
+    serverSocket.bind(new InetSocketAddress("127.0.0.1", 0))
+    final val port = serverSocket.getLocalPort
+    private final val thread = new Thread(new Runnable() {
+        override def run() =
+            mainLoop();
+    })
+
+    thread.setDaemon(true)
+    thread.setName("ToyHttp")
+    thread.start()
+
+    def stop(): Unit = {
+        try serverSocket.close() catch { case e: IOException => }
+        try thread.interrupt() catch { case NonFatal(e) => }
+        thread.join()
+    }
+
+    @tailrec
+    private def mainLoop(): Unit = {
+        val done = try {
+            val socket = serverSocket.accept()
+
+            try handleRequest(socket)
+            catch {
+                case _: EOFException =>
+                case e: IOException =>
+                    System.err.println(s"error handling http request: ${e.getClass.getName}: ${e.getMessage} ${e.getStackTrace.mkString("\n")}")
+            }
+            false
+        } catch {
+            case e: java.net.SocketException =>
+                true
+        }
+        if (!done)
+            mainLoop()
+    }
+
+    private def handleRequest(socket: Socket): Unit = {
+        val in = socket.getInputStream
+        val out = socket.getOutputStream
+        try {
+            // HTTP requests look like this:
+            // GET /path/here HTTP/1.0
+            // SomeHeader: bar
+            // OtherHeader: foo
+            // \r\n
+            val reader = new BufferedReader(new java.io.InputStreamReader(in))
+            val path = parsePath(reader)
+            val header = parseHeader(reader)
+            //System.err.println(s"request path '$path' headers $header")
+            val response = handler(Request(path, header))
+            //System.err.println(s"response $response")
+            sendResponse(out, response)
+        } finally {
+            in.close()
+            out.close()
+        }
+    }
+
+    private def parseHeader(reader: BufferedReader): Map[String, String] = {
+
+        def readHeaders(sofar: Map[String, String]): Map[String, String] = {
+            val line = reader.readLine()
+            val colon = line.indexOf(':')
+            if (colon > 0) {
+                val name = line.substring(0, colon).toLowerCase()
+                val value = line.substring(colon + 1).replaceAll("^[ \t]+", "")
+                readHeaders(sofar + (name -> value))
+            } else {
+                sofar
+            }
+        }
+
+        readHeaders(Map.empty)
+    }
+
+    private def parsePath(reader: BufferedReader): String = {
+        val methodPathProto = reader.readLine().split(" +")
+        val method = methodPathProto(0)
+        val path = methodPathProto(1)
+
+        path
+    }
+
+    private def codeText(code: Int) = code match {
+        case 200 => "OK"
+        case 404 => "Not Found"
+        case 500 => "Internal Server Error"
+        case _ => throw new RuntimeException(s"add text for $code")
+    }
+
+    private def sendResponse(out: OutputStream, response: Response): Unit = {
+        //val stuff = new java.io.ByteArrayOutputStream
+        //val writer = new PrintWriter(new OutputStreamWriter(stuff, StandardCharsets.UTF_8))
+        val writer = new PrintWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8))
+        val dateFormat = new SimpleDateFormat("E, d MMM yyyy HH:mm:ss 'GMT'", Locale.US);
+        dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
+
+        writer.append(s"HTTP/1.1 ${response.code} ${codeText(response.code)}\r\n")
+        writer.append(s"Date: ${dateFormat.format(new Date)}\r\n")
+        writer.append(s"Content-Type: ${response.contentType}; charset=utf-8\r\n")
+        val bytes = response.body.getBytes("UTF-8")
+        writer.append(s"Content-Length: $bytes\r\n")
+        writer.append("\r\n")
+        writer.append(response.body)
+        writer.flush()
+    }
+}
+
+object ToyHttp {
+    def apply(handler: Request => Response): ToyHttp =
+        new ToyHttp(handler)
+
+    final case class Request(path: String, headers: Map[String, String])
+    final case class Response(code: Int, contentType: String, body: String)
+}