Package: trapperkeeper-webserver-jetty9-clojure / 1.7.0-2

jetty-9.4-compat Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
From: Matthaus Owens <matthaus@puppet.com>
Date: Wed, 22 Feb 2017 10:42:28 -0800
Subject: [PATCH] (TK-369) Update jetty dependency to 9.4.1

This commit updates jetty to 9.4.1, which requires several changes to
tests and the config and core namespaces. Here are a list of changes and
why they were made:

Code changes

* Manually start the request logger when set
  This change
    https://github.com/eclipse/jetty.project/commit/34a8da2ba26a806413f06ab18fdfad535de45cfa
  seems to have caused logging to no longer work without starting here.
  Starting after the server was started did not do the right thing.

* GzipHandler changes
  GzipHandler moved into a new package, into jetty-server from
  jetty-servlets and some of the setup methods changed, notably
  around excluding mime types.

* ProxyServlet changes
  Several methods in the ProxyServlet and AbstractProxyServlet were
  deprecated, so the methods being proxied also have been updated.

* Enable client redirects
  The http connection used for proxies began clearing all handlers in
  https://github.com/eclipse/jetty.project/commit/c7cff6ec7e28f74156d8f78bada08ae5c9cabf48#diff-571b48f4ff5fbf0a57c41ad7c7ac5dedR297
  so we must manually add the redirect handler back on to correctly
  handle 302s

* Disable symlinks
  In https://github.com/eclipse/jetty.project/commit/d8e6331434fbb6025301f06f03230c6f6cad7676,
  symlink aliases were allowed by default on all unix platforms, so we
  clear aliases when follow-symlinks is false.

Test changes

* 431 now returned for headers that are too long instead of 413

* SSLException thrown instead of ConnectException for some errors

* URIUtil decodePath changes
  decodePath was changed during a refactor. Path traversal is still
  protected against, but the characters returned have changed, and
  additionally semicolons no longer terminate parsing.
---
 CHANGELOG.md                                       |  6 ++++
 project.clj                                        |  2 +-
 .../services/webserver/jetty9_config.clj           |  1 +
 .../services/webserver/jetty9_core.clj             | 35 ++++++++++++----------
 .../webserver/jetty9_default_config_test.clj       | 13 ++++----
 .../services/webserver/jetty9_service_test.clj     | 12 +++++---
 .../webserver/normalized_uri_helpers_test.clj      |  6 ++--
 7 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 63268c6..74a91fe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 2.0.0
+
+The is a major release
+
+* [TK-369](https://tickets.puppetlabs.com/browse/TK-369) Move Jetty dependency to 9.4.1
+
 ## 1.7.0
 
 This is a feature and bugfix release.
diff --git a/project.clj b/project.clj
index 3826cbd..3bf7ba0 100644
--- a/project.clj
+++ b/project.clj
@@ -1,4 +1,4 @@
-(def jetty-version "9.2.10.v20150310")
+(def jetty-version "9.4.1.v20170120")
 
 (defproject puppetlabs/trapperkeeper-webserver-jetty9 "1.7.0"
   :description "A jetty9-based webserver implementation for use with the puppetlabs/trapperkeeper service framework."
diff --git a/src/puppetlabs/trapperkeeper/services/webserver/jetty9_config.clj b/src/puppetlabs/trapperkeeper/services/webserver/jetty9_config.clj
index 20e564a..21a6085 100644
--- a/src/puppetlabs/trapperkeeper/services/webserver/jetty9_config.clj
+++ b/src/puppetlabs/trapperkeeper/services/webserver/jetty9_config.clj
@@ -448,6 +448,7 @@
     (.setFileName logger (:access-log-config config))
     (.setQuiet logger true)
     (.setRequestLog handler logger)
+    (.start logger)
     handler))
 
 (defn maybe-init-log-handler
diff --git a/src/puppetlabs/trapperkeeper/services/webserver/jetty9_core.clj b/src/puppetlabs/trapperkeeper/services/webserver/jetty9_core.clj
index 566b445..99c9885 100644
--- a/src/puppetlabs/trapperkeeper/services/webserver/jetty9_core.clj
+++ b/src/puppetlabs/trapperkeeper/services/webserver/jetty9_core.clj
@@ -3,22 +3,22 @@
                                      HttpConfiguration HttpConnectionFactory
                                      ConnectionFactory AbstractConnectionFactory)
            (org.eclipse.jetty.server.handler AbstractHandler ContextHandler HandlerCollection
-                                             ContextHandlerCollection AllowSymLinkAliasChecker StatisticsHandler HandlerWrapper)
+                                             ContextHandlerCollection AllowSymLinkAliasChecker
+                                             StatisticsHandler HandlerWrapper)
+           (org.eclipse.jetty.server.handler.gzip GzipHandler)
            (org.eclipse.jetty.util.resource Resource)
            (org.eclipse.jetty.util.thread QueuedThreadPool)
            (org.eclipse.jetty.util.ssl SslContextFactory)
            (javax.servlet.http HttpServletResponse)
            (java.util.concurrent TimeoutException)
-           (org.eclipse.jetty.servlets.gzip GzipHandler)
            (org.eclipse.jetty.servlet ServletContextHandler ServletHolder DefaultServlet)
            (org.eclipse.jetty.webapp WebAppContext)
-           (java.util HashSet)
            (org.eclipse.jetty.http MimeTypes HttpHeader HttpHeaderValue)
            (javax.servlet Servlet ServletContextListener)
            (org.eclipse.jetty.proxy ProxyServlet)
            (java.net URI)
            (java.security Security)
-           (org.eclipse.jetty.client HttpClient)
+           (org.eclipse.jetty.client HttpClient RedirectProtocolHandler)
            (clojure.lang Atom)
            (java.lang.management ManagementFactory)
            (org.eclipse.jetty.jmx MBeanContainer)
@@ -382,7 +382,7 @@
                  (.startsWith % "video/"))
             (MimeTypes/getKnownMimeTypes))
     (conj "application/compress" "application/zip" "application/gzip" "text/event-stream")
-    (HashSet.)))
+    (into-array)))
 
 (defn- gzip-handler
   "Given a handler, wrap it with a GzipHandler that will compress the response
@@ -390,8 +390,7 @@
   [handler]
   (doto (GzipHandler.)
     (.setHandler handler)
-    (.setMimeTypes (gzip-excluded-mime-types))
-    (.setExcludeMimeTypes true)))
+    (.setExcludedMimeTypes (gzip-excluded-mime-types))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Handler Helper Functions
@@ -437,7 +436,7 @@
                                     (:ssl-config options)))
         {:keys [request-buffer-size idle-timeout]} options]
     (proxy [ProxyServlet] []
-      (rewriteURI [req]
+      (rewriteTarget [req]
         (let [query (.getQueryString req)
               scheme (let [target-scheme (:scheme options)]
                        (condp = target-scheme
@@ -458,8 +457,8 @@
                                  (codec/url-decode (str query))
                                  nil)]
             (if-let [rewrite-uri-callback-fn (:rewrite-uri-callback-fn options)]
-              (rewrite-uri-callback-fn target-uri req)
-              target-uri))))
+              (str (rewrite-uri-callback-fn target-uri req))
+              (str target-uri)))))
 
       (newHttpClient []
         (let [client (if custom-ssl-ctxt-factory
@@ -477,22 +476,25 @@
               timeout (when idle-timeout
                         (* 1000 idle-timeout))]
           (if (:follow-redirects options)
-            (.setFollowRedirects client true)
+            (do
+              (.setFollowRedirects client true)
+              (.put (.getProtocolHandlers client) (RedirectProtocolHandler. client)))
             (.setFollowRedirects client false))
           (when timeout
             (.setIdleTimeout client timeout))
           client))
 
-      (customizeProxyRequest [proxy-req req]
+      (sendProxyRequest [req resp proxy-req]
         (if-let [callback-fn (:callback-fn options)]
-         (callback-fn proxy-req req)))
+         (callback-fn proxy-req req))
+       (proxy-super sendProxyRequest req resp proxy-req))
 
       ;; The implementation of onResponseFailure is duplicated heavily from:
       ;; https://github.com/eclipse/jetty.project/blob/jetty-9.2.10.v20150310/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java#L576-L607
       ;; The only significant difference is that a 'failure-callback-fn', if
       ;; defined in options, is invoked just prior to completing the async
       ;; context for cases that the response was not already committed upstream.
-      (onResponseFailure [req resp proxy-resp failure]
+      (onProxyResponseFailure [req resp proxy-resp failure]
         (do
           (let [request-id    (.getRequestId this req)
                 async-context (.getAsyncContext req)]
@@ -702,8 +704,9 @@
          enable-trailing-slash-redirect? (:enable-trailing-slash-redirect? options)
          normalize-request-uri? (:normalize-request-uri? options)]
      (.setBaseResource handler (Resource/newResource base-path))
-     (when follow-links?
-       (.addAliasCheck handler (AllowSymLinkAliasChecker.)))
+     (if follow-links?
+       (.addAliasCheck handler (AllowSymLinkAliasChecker.))
+       (.clearAliasChecks handler))
      ;; register servlet context listeners (if any)
      (when-not (nil? context-listeners)
        (dorun (map #(.addEventListener handler %) context-listeners)))
diff --git a/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_default_config_test.clj b/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_default_config_test.clj
index 6cc3a25..eaf0d86 100644
--- a/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_default_config_test.clj
+++ b/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_default_config_test.clj
@@ -90,8 +90,11 @@ react accordingly."
 
 (def selector-thread-count
   "The number of selector threads that should be allocated per connector.  See:
-   https://github.com/eclipse/jetty.project/blob/jetty-9.2.10.v20150310/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java#L229"
-  (max 1 (min 4 (int (/ (ks/num-cpus) 2)))))
+   https://github.com/eclipse/jetty.project/blob/jetty-9.4.1.v20170120/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java#L223
+   and
+   https://github.com/eclipse/jetty.project/blob/jetty-9.4.1.v20170120/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java#L403-L408
+   The number of selectors is twice the number of selector threads."
+  (* 2 (max 1 (min 4 (int (/ (ks/num-cpus) 2))))))
 
 (def acceptor-thread-count
   "The number of acceptor threads that should be allocated per connector.  See:
@@ -109,7 +112,7 @@ react accordingly."
     (is (= acceptor-thread-count (.getAcceptors connector))
         "Unexpected default for 'acceptor-threads' and 'ssl-acceptor-threads'")
     (is (= selector-thread-count
-           (.getSelectorCount (.getSelectorManager connector)))
+           (* 2 (.getSelectorCount (.getSelectorManager connector))))
         "Unexpected default for 'selector-threads' and 'ssl-selector-threads'")))
 
 (defn get-max-threads-for-server
@@ -177,7 +180,7 @@ react accordingly."
     (dotimes [x 2]
       (let [connectors       (inc x)
             required-threads (calculate-minimum-required-threads connectors)]
-        (testing (str "server with too few threads for " x " connector(s) "
+        (testing (str "server with too few threads for " connectors " connector(s) "
                       "fail(s) to start with expected error")
           (let [server (-> required-threads
                            dec
@@ -186,7 +189,7 @@ react accordingly."
                                   (insufficient-threads-msg server)
                                   (tk-log-testutils/with-test-logging
                                    (.start server))))))
-        (testing (str "server with minimum required threads for " x
+        (testing (str "server with minimum required threads for " connectors
                       "connector(s) start(s) successfully")
           (let [server (get-server required-threads connectors)]
             (try
diff --git a/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_service_test.clj b/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_service_test.clj
index d5994d0..de3eb67 100644
--- a/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_service_test.clj
+++ b/test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_service_test.clj
@@ -5,7 +5,8 @@
            (java.net BindException)
            (java.nio.file Paths Files)
            (java.nio.file.attribute FileAttribute)
-           (appender TestListAppender))
+           (appender TestListAppender)
+           (javax.net.ssl SSLException))
   (:require [clojure.test :refer :all]
             [puppetlabs.http.client.async :as async]
             [puppetlabs.http.client.common :as http-client-common]
@@ -69,6 +70,8 @@
     (throw (IllegalStateException. "Expected SSL Exception to be thrown!"))
     (catch ConnectionClosedException e#
       true)
+    (catch SSLException e#
+      true)
     (catch IOException e#
       (if (= "Connection reset by peer" (.getMessage e#))
         true
@@ -529,7 +532,8 @@
       (tk-app/stop app))))
 
 (deftest large-request-test
-  (testing (str "request to Jetty fails with a 413 error if the request header "
+  ;; This changed from 413 to 431 in https://github.com/eclipse/jetty.project/commit/e53ea55f480a959a2f1f5e2dbdbfc689d61c94a6
+  (testing (str "request to Jetty fails with a 431 error if the request header "
                 "is too large and a larger one is not set")
     (with-app-with-config app
       [jetty9-service
@@ -538,7 +542,7 @@
       (tk-log-testutils/with-test-logging
        (let [response (http-get "http://localhost:8080/hi_world" {:headers {"Cookie" absurdly-large-cookie}
                                                                   :as :text})]
-         (is (= (:status response) 413))))))
+         (is (= (:status response) 431))))))
 
   (testing (str "request to Jetty succeeds with a large cookie if the request header "
                 "size is properly set")
@@ -804,7 +808,7 @@
        hello-webservice]
       jetty-ssl-pem-config
       (is (thrown?
-           ConnectionClosedException
+           SSLException
            (http-get "https://localhost:8081/hi_world" (merge default-options-for-https-client
                                                               {:ssl-protocols ["SSLv3"]}))))))
   (testing "SSLv3 is supported when configured"
diff --git a/test/clj/puppetlabs/trapperkeeper/services/webserver/normalized_uri_helpers_test.clj b/test/clj/puppetlabs/trapperkeeper/services/webserver/normalized_uri_helpers_test.clj
index 59ce7fb..a81db95 100644
--- a/test/clj/puppetlabs/trapperkeeper/services/webserver/normalized_uri_helpers_test.clj
+++ b/test/clj/puppetlabs/trapperkeeper/services/webserver/normalized_uri_helpers_test.clj
@@ -33,7 +33,7 @@
   (testing (str "non-percent encoded parameters in uri path segments are "
                 "chopped off after normalization")
     (is (= "/foo" (normalize-uri-path-for-string "/foo;foo=chump")))
-    (is (= "/foo/bar" (normalize-uri-path-for-string
+    (is (= "/foo/bar/baz" (normalize-uri-path-for-string
                        "/foo/bar;bar=chocolate/baz;baz=bim"))))
   (testing (str "percent-encoded parameters in uri path segments are properly "
                 "decoded after normalization")
@@ -116,8 +116,8 @@
     ;;> or the surrogate pair ED A1 8C ED BE B4 into U+233B4.  Decoding
     ;;> invalid sequences may have security consequences or cause other
     ;;> problems.
-    (is (= "��" (normalize-uri-path-for-string "%C0%AE")))
-    (is (= "/foo/��/��" (normalize-uri-path-for-string "/foo/%C0%AE/%C0%AE")))))
+    (is (= "-64-82" (normalize-uri-path-for-string "%C0%AE")))
+    (is (= "/foo/-64-82/-64-82" (normalize-uri-path-for-string "/foo/%C0%AE/%C0%AE")))))
 
 (deftest normalize-uris-with-redundant-slashes-tests
   (testing "uris with redundant slashes are removed"