[GitHub] [tomcat] alpire commented on a change in pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
alpire commented on a change in pull request #176: CoyoteAdapter: fix
out-of-bounds read in checkNormalize
URL: https://github.com/apache/tomcat/pull/176#discussion_r299284882
##
File path: test/org/apache/catalina/connector/TestCoyoteAdapter.java
##
@@ -344,6 +345,29 @@ private void doTestNormalize(String input, String
expected) {
}
}
+@Test
+public void testCheckNormalize() {
+doTestCheckNormalize("/url", true);
+
+doTestCheckNormalize("", false);
+doTestCheckNormalize("..", false);
+doTestCheckNormalize("/.", false);
+doTestCheckNormalize("/..", false);
+doTestCheckNormalize("/./", false);
+doTestCheckNormalize("//", false);
+doTestCheckNormalize("/../", false);
+doTestCheckNormalize("\\", false);
+doTestCheckNormalize("\0", false);
+}
Review comment:
I split the tests into different functions and removed a few of them since
we want to focus the `checkNormalize()` tests to what is possible given its
usage.
I did not update the tests to go through `normalize` -> `convertURI` ->
`checkNormalize`, but I'd be happy to do this if you think that'd be more
appropriate here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] alpire commented on a change in pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
alpire commented on a change in pull request #176: CoyoteAdapter: fix
out-of-bounds read in checkNormalize
URL: https://github.com/apache/tomcat/pull/176#discussion_r299276651
##
File path: java/org/apache/catalina/connector/CoyoteAdapter.java
##
@@ -1271,6 +1276,11 @@ public static boolean checkNormalize(MessageBytes
uriMB) {
}
}
+// The URL must start with '/'
+if (c[start] != '/') {
+return false;
+}
+
// Check for ending with "/." or "/.."
Review comment:
I do have an input that triggers a 500 response if that check is removed:
```
: 202e 2e20 3020 0a .. 0 .
```
That input trigger the exception that's in the PR description.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] alpire commented on a change in pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
alpire commented on a change in pull request #176: CoyoteAdapter: fix
out-of-bounds read in checkNormalize
URL: https://github.com/apache/tomcat/pull/176#discussion_r299276368
##
File path: java/org/apache/catalina/connector/CoyoteAdapter.java
##
@@ -1252,6 +1252,11 @@ public static boolean checkNormalize(MessageBytes
uriMB) {
int pos = 0;
+// An empty URL is not acceptable
+if (start == end) {
+return false;
+}
+
// Check for '\' and 0
Review comment:
I do not have an input that would trigger a 500 if we removed that check. I
understand the performance consideration, so I'll remove it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] zhanhb commented on issue #175: Apply the suggestion in rfc7233
zhanhb commented on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507436005 Good job, there is no issue with the range request now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Align with 9.0.x. Add additional translations
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 1cee91d Align with 9.0.x. Add additional translations
1cee91d is described below
commit 1cee91d3e877e270248b2b3fe6c2280907a80039
Author: Mark Thomas
AuthorDate: Mon Jul 1 09:03:34 2019 +0100
Align with 9.0.x. Add additional translations
---
.../apache/catalina/servlets/LocalStrings_ru.properties | 17 +
1 file changed, 17 insertions(+)
diff --git a/java/org/apache/catalina/servlets/LocalStrings_ru.properties
b/java/org/apache/catalina/servlets/LocalStrings_ru.properties
new file mode 100644
index 000..7e629da
--- /dev/null
+++ b/java/org/apache/catalina/servlets/LocalStrings_ru.properties
@@ -0,0 +1,17 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF 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.
+
+cgiServlet.expandFail=Невозможно развернуть скрипт [{0}] в [{1}]
+cgiServlet.runInvalidStatus=Неверный статус [{0}]
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 7.0.x updated (0ef94ce -> 07b0f95)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 0ef94ce Align with 8.5.x
new 7adb47a Align with 8.5.x. Add additional translations
new 07b0f95 Align with 8.5.x. Remove unused keys, update/add translations
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/catalina/servlets/DefaultServlet.java | 4 +--
.../catalina/servlets/LocalStrings.properties | 17 -
.../catalina/servlets/LocalStrings_es.properties | 24 +++--
.../catalina/servlets/LocalStrings_fr.properties | 40 +-
.../catalina/servlets/LocalStrings_ja.properties | 35 +++
.../servlets}/LocalStrings_ru.properties | 3 +-
6 files changed, 84 insertions(+), 39 deletions(-)
copy java/org/apache/{tomcat/websocket/pojo =>
catalina/servlets}/LocalStrings_ru.properties (83%)
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Align with 8.5.x. Remove unused keys, update/add translations
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 07b0f952b4d5cd5db10780bff1c78c2d8f03577a
Author: Mark Thomas
AuthorDate: Mon Jul 1 09:23:44 2019 +0100
Align with 8.5.x. Remove unused keys, update/add translations
---
java/org/apache/catalina/servlets/DefaultServlet.java | 4 ++--
.../apache/catalina/servlets/LocalStrings.properties | 17 ++---
.../catalina/servlets/LocalStrings_es.properties | 17 ++---
.../catalina/servlets/LocalStrings_fr.properties | 18 ++
.../catalina/servlets/LocalStrings_ja.properties | 15 ---
5 files changed, 32 insertions(+), 39 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 8171ac0..6821f66 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -337,7 +337,7 @@ public class DefaultServlet extends HttpServlet {
}
if (resources == null) {
-throw new UnavailableException("No resources");
+throw new
UnavailableException(sm.getString("defaultServlet.noResources"));
}
if (getServletConfig().getInitParameter("showServerInfo") != null) {
@@ -2328,7 +2328,7 @@ public class DefaultServlet extends HttpServlet {
return e;
}
if (skipped < start) {
-return new IOException(sm.getString("defaultservlet.skipfail",
+return new IOException(sm.getString("defaultServlet.skipfail",
Long.valueOf(skipped), Long.valueOf(start)));
}
diff --git a/java/org/apache/catalina/servlets/LocalStrings.properties
b/java/org/apache/catalina/servlets/LocalStrings.properties
index ff2eff7..22adaa8 100644
--- a/java/org/apache/catalina/servlets/LocalStrings.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings.properties
@@ -38,20 +38,15 @@ cgiServlet.runStdErrFail=I/O problems with stderr
defaultServlet.blockExternalEntity=Blocked access to external entity with
publicId [{0}] and systemId [{0}]
defaultServlet.blockExternalEntity2=Blocked access to external entity with
name [{0}], publicId [{1}], baseURI [{2}] and systemId [{3}]
defaultServlet.blockExternalSubset=Blocked access to external subset with name
[{0}] and baseURI [{1}]
-defaultServlet.missingResource=The requested resource ({0}) is not available
-
-defaultservlet.directorylistingfor=Directory Listing for:
-defaultservlet.files=Files:
-defaultservlet.skipfail=Only skipped [{0}] bytes when [{1}] were requested
-defaultservlet.subdirectories=Subdirectories:
-defaultservlet.upto=Up to:
+defaultServlet.missingResource=The requested resource [{0}] is not available
+defaultServlet.noResources=No static resources were found
+defaultServlet.skipfail=Read failed because only [{0}] bytes were available
but needed to skip [{1}] bytes to reach the start of the requested range
directory.filename=Filename
directory.lastModified=Last Modified
-directory.parent=Up To {0}
+directory.parent=Up To [{0}]
directory.size=Size
-directory.title=Directory Listing For {0}
-directory.version=Tomcat Catalina version 4.0
+directory.title=Directory Listing For [{0}]
-webdavservlet.enternalEntityIgnored=The request included a reference to an
external entity with PublicID {0} and SystemID {1} which was ignored
+webdavservlet.enternalEntityIgnored=The request included a reference to an
external entity with PublicID [{0}] and SystemID [{1}] which was ignored
webdavservlet.jaxpfailed=JAXP initialization failed
diff --git a/java/org/apache/catalina/servlets/LocalStrings_es.properties
b/java/org/apache/catalina/servlets/LocalStrings_es.properties
index af9bceb..c96df05 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_es.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_es.properties
@@ -22,20 +22,15 @@ cgiServlet.runOutputStreamFail=Errores I/O cerrando el
flujo de salida
cgiServlet.runReaderInterrupt=Detenido esperando por el hilo lector stderr
cgiServlet.runStdErrFail=Problemas de I/O con stderr
-defaultServlet.missingResource=El recurso requerido {0} no se encuentra
disponible
-
-defaultservlet.directorylistingfor=Listado de Directorio para:
-defaultservlet.files=Ficheros:
-defaultservlet.skipfail=Sólo se han saltado [{0}] cuando se requirieron [{1}]
-defaultservlet.subdirectories=Subdirectorios:
-defaultservlet.upto=Atrás a:
+defaultServlet.blockExternalSubset=Se bloqueó el acceso al subconjunt externo
con nombre [{0}] y URI base [{1}]\n
+defaultServlet.missingResource=El recurso requerido [{0}] no se encuentra
disponible
+defaultServlet.skipfail=Sólo se han saltado [{0}] cuando se requirieron [{1}]
directory.filename=Nombre de Fichero:
directory.lastModified=Última Modificación
-directory.parent=Atrás
[tomcat] 01/02: Align with 8.5.x. Add additional translations
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 7adb47ae0c29034e0d3a8ec38655fa6aaef6e711
Author: Mark Thomas
AuthorDate: Mon Jul 1 09:03:54 2019 +0100
Align with 8.5.x. Add additional translations
---
.../catalina/servlets/LocalStrings_es.properties | 9 +
.../catalina/servlets/LocalStrings_fr.properties | 22 ++
.../catalina/servlets/LocalStrings_ja.properties | 20
...gs_fr.properties => LocalStrings_ru.properties} | 15 ++-
4 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/catalina/servlets/LocalStrings_es.properties
b/java/org/apache/catalina/servlets/LocalStrings_es.properties
index 02b3014..af9bceb 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_es.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_es.properties
@@ -13,6 +13,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+cgiServlet.expandFail=Fallo al expandir el script [{0}] en [{1}]\n
+cgiServlet.expandOk=Expandiendo script en el path [{0}] hacia [{1}]\n
+cgiServlet.find.location=Buscando archivo en [{0}]\n
+cgiServlet.runHeaderReaderFail=Problemas de I/O cerrando la cabecera del
lector
+cgiServlet.runInvalidStatus=Estado inválido [{0}]
+cgiServlet.runOutputStreamFail=Errores I/O cerrando el flujo de salida
+cgiServlet.runReaderInterrupt=Detenido esperando por el hilo lector stderr
+cgiServlet.runStdErrFail=Problemas de I/O con stderr
+
defaultServlet.missingResource=El recurso requerido {0} no se encuentra
disponible
defaultservlet.directorylistingfor=Listado de Directorio para:
diff --git a/java/org/apache/catalina/servlets/LocalStrings_fr.properties
b/java/org/apache/catalina/servlets/LocalStrings_fr.properties
index 8845a4f..fba2435 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_fr.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_fr.properties
@@ -13,6 +13,28 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+cgiServlet.emptyEnvVarName=la nom de variable d'environnement est vide dans le
paramètre d'initialisation [environment-variable-]
+cgiServlet.expandCloseFail=Impossible de fermer le flux d''entrée du script
avec le chemin [{0}]
+cgiServlet.expandCreateDirFail=Echec de la création du répertoire de
destination [{0}] pour la décompression du script
+cgiServlet.expandDeleteFail=Impossible d''effacer le fichier [{0}] suite à une
IOException pendant la décompression
+cgiServlet.expandFail=Impossible de faire l''expansion du script au chemin
[{0}] vers [{1}]
+cgiServlet.expandNotFound=Impossible de décompresser [{0}] car il n''a pas été
trouvé
+cgiServlet.expandOk=Extrait le script du chemin [{0}] vers [{1}]
+cgiServlet.find.found=Trouvé le CGI: nom [{0}], chemin [{1}], nom de script
[{2}] et nom du CGI [{3}]
+cgiServlet.find.location=Recherche d''un fichier en [{0}]
+cgiServlet.find.path=Script CGI demandé au chemin [{0}] relatif au CGI à [{1}]
+cgiServlet.invalidArgumentDecoded=Les paramètres de ligne de commande décodés
[{0}] ne correspondent pas au modèle cmdLineArgumentsDecoded configuré [{1}]
+cgiServlet.invalidArgumentEncoded=Les paramètres de ligne de commande encodés
[{0}] ne correspondent pas au modèle cmdLineArgumentsEncoded configuré [{1}]
+cgiServlet.runBadHeader=Mauvaise ligne d''en-tête [{0}]
+cgiServlet.runFail=Problèmes d'IO lors de l'exécution du CGI
+cgiServlet.runHeaderReaderFail=Problème d'E/S lors de la fermeture du lecteur
de headers
+cgiServlet.runInvalidStatus=Statut invalide [{0}]
+cgiServlet.runOutputStreamFail=Problème d'E/S à la fermeture du flux de sortie
+cgiServlet.runReaderInterrupt=Interrompu pendant l'attente du thread de
lecture de la sortie d'erreur (stderr reader thread)
+cgiServlet.runStdErr=ligne stderr: [{0}]
+cgiServlet.runStdErrCount=Reçues [{0}] lignes sur le stderr
+cgiServlet.runStdErrFail=Problème d'entrée sortie pour le stderr
+
defaultservlet.directorylistingfor=Liste du répertoire pour :
defaultservlet.files=Fichiers:
defaultservlet.subdirectories=Sous-répertoires:
diff --git a/java/org/apache/catalina/servlets/LocalStrings_ja.properties
b/java/org/apache/catalina/servlets/LocalStrings_ja.properties
index 45e7ae9..85ad7c0 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_ja.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_ja.properties
@@ -13,6 +13,26 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+cgiServlet.emptyEnvVarName=初期化パラメータの空の環境変数名[環境変数]
+cgiServlet.expandCloseFail=パス[{0}]のスクリプトの入力ストリームを閉じることができませんでした。
+cgiServlet.expandCreateDirFail=スクリプトの展開先ディレクトリ[{0}]の作成に失敗しました。
+cgiServlet.expandDeleteFail=拡張中にIOExceptionの後に[{0}]でファイルを削除できませんでした
[GitHub] [tomcat] markt-asf commented on issue #175: Apply the suggestion in rfc7233
markt-asf commented on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507409540 I think everything is complete now. Feel free to re-open if you spot something I missed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #175: Apply the suggestion in rfc7233
markt-asf closed pull request #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/05: Make partial PUT processing optional but still enabled by default
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit bb497d62e1405c8ba56d1910672d8c476e0b8dba
Author: Mark Thomas
AuthorDate: Mon Jul 1 13:28:31 2019 +0100
Make partial PUT processing optional but still enabled by default
---
conf/web.xml | 5 +
.../apache/catalina/servlets/DefaultServlet.java | 22 ++
webapps/docs/changelog.xml | 6 ++
webapps/docs/default-servlet.xml | 5 +
4 files changed, 38 insertions(+)
diff --git a/conf/web.xml b/conf/web.xml
index 4106441..9c0a248 100644
--- a/conf/web.xml
+++ b/conf/web.xml
@@ -104,6 +104,11 @@
+
+
+
+
+
default
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index d5b9ab0..5ddfcb8 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -271,6 +271,12 @@ public class DefaultServlet extends HttpServlet {
*/
protected transient SortManager sortManager;
+/**
+ * Flag that indicates whether partial PUTs are permitted.
+ */
+private boolean allowPartialPut = true;
+
+
// - Public Methods
/**
@@ -371,6 +377,10 @@ public class DefaultServlet extends HttpServlet {
sortManager = new SortManager(sortDirectoriesFirst);
}
}
+
+if (getServletConfig().getInitParameter("allowPartialPut") != null) {
+allowPartialPut =
Boolean.parseBoolean(getServletConfig().getInitParameter("allowPartialPut"));
+}
}
private CompressionFormat[] parseCompressionFormats(String precompressed,
String gzip) {
@@ -1444,6 +1454,18 @@ public class DefaultServlet extends HttpServlet {
HttpServletResponse response,
WebResource resource) throws IOException {
+if (!"GET".equals(request.getMethod())) {
+// RFC 7233#3.1 clarifies the intention of RFC 2616 was to only
+// allow Range headers on GET requests. However, many people
+// incorrectly read RFC 2616#14.35.1 as allowing partial PUT and
+// implemented. Tomcat was one such implementation. It is
optionally
+// allowed to retain compatibility with clients that use it.
+if (!allowPartialPut || !"PUT".equals(request.getMethod())) {
+return FULL;
+}
+}
+
+
// Checking If-Range
String headerValue = request.getHeader("If-Range");
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3c2105c..14ab5f8 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -59,6 +59,12 @@
When comparing a date from a If-Range header, an exact
match is required. Based on a pull request by zhanhb. (markt)
+
+Add an option to the default servlet to disable processing of PUT
+requests with Range headers as partial PUTs. The default behaviour
+(processing as partial PUT) is unchanged. Based on a pull request by
+zhanhb. (markt)
+
diff --git a/webapps/docs/default-servlet.xml b/webapps/docs/default-servlet.xml
index a515f73..cd7d30e 100644
--- a/webapps/docs/default-servlet.xml
+++ b/webapps/docs/default-servlet.xml
@@ -201,6 +201,11 @@ Tomcat.
Should the server list all directories before all files. [false]
+
+Should the server treat an HTTP PUT request with a Range header as a
+partial PUT? Note that RFC 7233 clarified that Range headers are only
+valid for GET requests. [true]
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 05/05: Improve parsing of Content-Range headers
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d1f58003a97af79df452cdbe5e94052acc4b7188
Author: Mark Thomas
AuthorDate: Mon Jul 1 21:16:15 2019 +0100
Improve parsing of Content-Range headers
---
.../apache/catalina/servlets/DefaultServlet.java | 57 +++
.../tomcat/util/http/parser/ContentRange.java | 108
.../org/apache/tomcat/util/http/parser/Ranges.java | 2 +-
.../catalina/servlets/TestDefaultServletPut.java | 185 +
.../apache/catalina/startup/SimpleHttpClient.java | 10 ++
webapps/docs/changelog.xml | 9 +-
6 files changed, 339 insertions(+), 32 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index a87f4ce..c217cf6 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -80,6 +80,7 @@ import org.apache.catalina.util.URLEncoder;
import org.apache.catalina.webresources.CachedResource;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.http.ResponseUtil;
+import org.apache.tomcat.util.http.parser.ContentRange;
import org.apache.tomcat.util.http.parser.Ranges;
import org.apache.tomcat.util.res.StringManager;
import org.apache.tomcat.util.security.Escape;
@@ -151,6 +152,8 @@ public class DefaultServlet extends HttpServlet {
*/
protected static final ArrayList FULL = new ArrayList<>();
+private static final Range IGNORE = new Range();
+
/**
* MIME multipart separation string
*/
@@ -612,6 +615,11 @@ public class DefaultServlet extends HttpServlet {
Range range = parseContentRange(req, resp);
+if (range == null) {
+// Processing error. parseContentRange() set the error code
+return;
+}
+
InputStream resourceInputStream = null;
try {
@@ -619,11 +627,11 @@ public class DefaultServlet extends HttpServlet {
// resource - create a temp. file on the local filesystem to
// perform this operation
// Assume just one range is specified for now
-if (range != null) {
+if (range == IGNORE) {
+resourceInputStream = req.getInputStream();
+} else {
File contentFile = executePartialPut(req, range, path);
resourceInputStream = new FileInputStream(contentFile);
-} else {
-resourceInputStream = req.getInputStream();
}
if (resources.write(path, resourceInputStream, true)) {
@@ -1383,7 +1391,9 @@ public class DefaultServlet extends HttpServlet {
*
* @param request The servlet request we are processing
* @param response The servlet response we are creating
- * @return the range object
+ * @return the partial content-range, {@code null} if the content-range
+ * header was invalid or {@code #IGNORE} if there is no header to
+ * process
* @throws IOException an IO error occurred
*/
protected Range parseContentRange(HttpServletRequest request,
@@ -1391,45 +1401,37 @@ public class DefaultServlet extends HttpServlet {
throws IOException {
// Retrieving the content-range header (if any is specified
-String rangeHeader = request.getHeader("Content-Range");
+String contentRangeHeader = request.getHeader("Content-Range");
-if (rangeHeader == null || !allowPartialPut) {
-return null;
+if (contentRangeHeader == null) {
+return IGNORE;
}
-// bytes is the only range unit supported
-if (!rangeHeader.startsWith("bytes")) {
+if (!allowPartialPut) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
-rangeHeader = rangeHeader.substring(6).trim();
-
-int dashPos = rangeHeader.indexOf('-');
-int slashPos = rangeHeader.indexOf('/');
+ContentRange contentRange = ContentRange.parse(new
StringReader(contentRangeHeader));
-if (dashPos == -1) {
+if (contentRange == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
-if (slashPos == -1) {
+
+// bytes is the only range unit supported
+if (!contentRange.getUnits().equals("bytes")) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return null;
}
+// TODO: Remove the internal representation and use Ranges
+// Convert to internal representation
Range range = new Range();
-
-try {
-range.start = Long.parseLong(rangeHeader.substring(0, dashPos));
-range.end =
[tomcat] 03/05: Fix configuration of partial PUT
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f9afd7a0be49f974c50db49cd9a12916c9fff4b2
Author: Mark Thomas
AuthorDate: Mon Jul 1 17:08:06 2019 +0100
Fix configuration of partial PUT
---
java/org/apache/catalina/servlets/DefaultServlet.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 5d3cc79..a87f4ce 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -1393,8 +1393,9 @@ public class DefaultServlet extends HttpServlet {
// Retrieving the content-range header (if any is specified
String rangeHeader = request.getHeader("Content-Range");
-if (rangeHeader == null)
+if (rangeHeader == null || !allowPartialPut) {
return null;
+}
// bytes is the only range unit supported
if (!rangeHeader.startsWith("bytes")) {
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/05: Remove unnecessary test
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 873b25b0c24b71a04b22439487e401b74decb309
Author: Mark Thomas
AuthorDate: Mon Jul 1 16:15:47 2019 +0100
Remove unnecessary test
---
java/org/apache/catalina/servlets/DefaultServlet.java | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 5ddfcb8..5d3cc79 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -1454,17 +1454,9 @@ public class DefaultServlet extends HttpServlet {
HttpServletResponse response,
WebResource resource) throws IOException {
-if (!"GET".equals(request.getMethod())) {
-// RFC 7233#3.1 clarifies the intention of RFC 2616 was to only
-// allow Range headers on GET requests. However, many people
-// incorrectly read RFC 2616#14.35.1 as allowing partial PUT and
-// implemented. Tomcat was one such implementation. It is
optionally
-// allowed to retain compatibility with clients that use it.
-if (!allowPartialPut || !"PUT".equals(request.getMethod())) {
-return FULL;
-}
-}
-
+// Range headers are only valid on GET requests. That implies they are
+// also valid on HEAD requests. This method is only called by doGet()
+// and doHead() so no need to check the request method.
// Checking If-Range
String headerValue = request.getHeader("If-Range");
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated (92585d3 -> d1f5800)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 92585d3 If-Range must use exact match on dates (allowing for resolution) new bb497d6 Make partial PUT processing optional but still enabled by default new 873b25b Remove unnecessary test new f9afd7a Fix configuration of partial PUT new 3992dc4 Remove completed TODO new d1f5800 Improve parsing of Content-Range headers The 5 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: conf/web.xml | 5 + .../apache/catalina/servlets/DefaultServlet.java | 72 .../tomcat/util/http/parser/ContentRange.java | 108 .../org/apache/tomcat/util/http/parser/Ranges.java | 2 +- .../catalina/servlets/TestDefaultServletPut.java | 185 + .../servlets/TestDefaultServletRangeRequests.java | 1 - .../apache/catalina/startup/SimpleHttpClient.java | 10 ++ webapps/docs/changelog.xml | 9 + webapps/docs/default-servlet.xml | 5 + 9 files changed, 367 insertions(+), 30 deletions(-) create mode 100644 java/org/apache/tomcat/util/http/parser/ContentRange.java create mode 100644 test/org/apache/catalina/servlets/TestDefaultServletPut.java - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 04/05: Remove completed TODO
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3992dc4559e8487f55723dee3e476d1faa1126a1
Author: Mark Thomas
AuthorDate: Mon Jul 1 17:12:01 2019 +0100
Remove completed TODO
---
test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java | 1 -
1 file changed, 1 deletion(-)
diff --git
a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
index da5d477..c50cc4b 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
+++ b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
@@ -37,7 +37,6 @@ import org.apache.tomcat.util.buf.ByteChunk;
@RunWith(Parameterized.class)
public class TestDefaultServletRangeRequests extends TomcatBaseTest {
-// TODO: Add a check for response headers
@Parameterized.Parameters(name = "{index} rangeHeader [{0}]")
public static Collection parameters() {
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #175: Apply the suggestion in rfc7233
markt-asf commented on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507304794 Thanks for the the review. I had mis-remembered how partial PUTs were implemented. I'll take another pass at the patch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [GitHub] [tomcat] alpire opened a new pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Mark, On 6/29/19 16:01, Mark Thomas wrote: > On 29/06/2019 02:26, GitBox wrote: >> alpire opened a new pull request #176: CoyoteAdapter: fix >> out-of-bounds read in checkNormalize URL: >> https://github.com/apache/tomcat/pull/176 >> >> >> On malformed requests, checkNormalize would throw an >> ArrayIndexOutOfBoundsException leading to a 500 response. This >> change fixes checkNormalize to return false instead of throwing >> exception on those inputs, and adds a few tests to check the new >> functionality. > > That there are URI sequences that can trigger this is certainly > something that we need to fix. > > On a slightly different topic, this got me thinking. > > checkNormalize() is a test that runs on every request. It is > essentially there to ensure that whatever character set has been > specified for the Connector is "safe". In this case "safe" means > when the bytes are converted to characters we don't get any > unexpected "." or "/" characters in the result that make the > character version of the URI non-normalized. > > Rather than run this test on every request, we could: - > pre-calculate the character sets we consider to be safe - throw an > exception if the user attempts to configure the Connector to use > one > > I think we could safely exclude any character set where any of the > following were not true: > > - 0x00 <-> '\u' - 0x2e <-> '.' - 0x2f <-> '/' - 0x5c <-> '\' > > We could create a unit test that maintains/checks the list of > "safe" character set canonical names. After creating the character > set in Connector.setURIEncoding(), if the canonical name of the > resulting character set is not in the safe list, throw an > exception. > > Then remove checkNormalize() and replace with a comment that > explains why the conversion is known to be safe. > > There are several loops through the URI in checkNormalize(). I > think - I'd need to test to confirm - that removing them would > provide a measurable performance benefit. I'm a little worried about edge cases that might cause a misread of a multibyte character into several single-byte characters that aren't supposed to be treated as such. Remember that an attacker is not obligated to follow the rules of sane e.g. UTF-8 or SHIFT-JS or whatever. They can claim one encoding and then send complete garbage down the line. We need to remain safe in all cases. So I think checking the string AFTER decoding is the only truly safe way to operate. Note that I can't think of a specific way to break this off the top of my head. What kind of performance benefit are you looking at, here? Certainly, every bit helps, but is the wire-to-servlet pipeline really that long at this point? - -chris -BEGIN PGP SIGNATURE- Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl0aIY8ACgkQHPApP6U8 pFhzrBAAwpIKVsiUIJiEQIEMsioeBSyShut/jncTjYNe1+JpdDWySDgQtuv6rzJm ImMsi7/Iutm+OTvPyX/0ErkueYTwsdQTMS3dAGnyC+naViLQ+/C5XsOLIisHJwUy suDJsVl5V0DbslXauhNiy+tsMzA8ipx87OneYO7gBMjcurLtMjvPiCnMvRS9eFah C5bU6Emu9NrEC40MI7Jw+Il/loUIDlLbCDIqDjGxMb/tA8N0wXmIoIIbC1iAXPpy fmSN4MyEUd0Xi4m/sv7kl71hmI09ivItHUtMdW89Tp8kvIEWUAnu3mDoLZGmw3+X YZIxhW+95g4HydOh7ODuJa7Bqg+csa0ZpQl5u4jnnwGZrLWLGZ/w+tu5P83qHBp6 Yy1hdPVrhfAgLMV1Rmzj+5Heo0LGe2PbKvgpg4DMAFSed3SOnnW/w3NNPTjFhEBD jiQi1TSXTZZQ6VRS/KWain4uQAWD2ouPipZl7XA8Uz+g0pEhh642s7LNUpbclH5y JZ02GRL5GFAcb9ZTn0RlCQgDz/xK9Lm5eVbHmWkdtHw9CUgqWF9nD6ZhWXEEI39Z vVQZboAZMlkijv3AJupjE6Q8PfPnXWaFdbtDqWMJQ+/c12xzPSGu+QcpgCYxyIfE 6kSig2FGR98lKXCE5bMMq6XuM1RNcHMyKUnOZ8NOxAhLfBbZcEw= =zgdi -END PGP SIGNATURE- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] zhanhb edited a comment on issue #175: Apply the suggestion in rfc7233
zhanhb edited a comment on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507295587 For a put request, usually it won't have a header 'Range', The header specified in rfc 2616 is 'Content-Range'. What will happen to a put request if header 'Range' is specified? Put the content in specified URI and then return partial content of the entity. We can see existed source in tomcat, either 201 or 204 is returned, both have no response body content. IMO, the third change can also be applied too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] zhanhb commented on issue #175: Apply the suggestion in rfc7233
zhanhb commented on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507295587 For a put request, usually it won't have a header 'Range', The header specified in rfc 2616 is 'Content-Range'. What will happen to a put request if header 'Range' is specified? Put the content in specified URI and then return partial content of the entity. We can see existed source in tomcat, either 201 or 204 is returned, both have no response body content. IMO, this change can also be applied too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [VOTE] Release Apache Tomcat Native 1.2.23
On Wed, Jun 26, 2019 at 3:20 PM Mark Thomas wrote: > Version 1.2.23 includes the following changes compared to 1.2.21 > > - Fix crashes associated with the use of a CRL > > - Add support for TLS key logging. Patch provided by John Kelly. > > - Make Windows 7 the default build target on Windows > > > Various other fixes and improvements. See the changelog for details. > > The proposed release artefacts can be found at [1], > and the build was done using tag [2]. > > The Apache Tomcat Native 1.2.23 release is > [X] Stable, go ahead and release > [ ] Broken because of ... > > Refreshed my environment. Did not test the new fixes and features. Rémy > Thanks, > > Mark > > > [1] > > https://dist.apache.org/repos/dist/dev/tomcat/tomcat-connectors/native/1.2.23 > [2] > > https://gitbox.apache.org/repos/asf?p=tomcat-native.git;a=commit;h=2f24cb4c2717f170755b74585f33802bf0f371c6 > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
[Bug 48392] jdbc-pool is not returning the proxied connection in resultSet and statement
https://bz.apache.org/bugzilla/show_bug.cgi?id=48392 --- Comment #9 from Dano George --- The Connection (by the connection pool) appears to be a logical connection, while the close() method is not used to close the connection, virtually, it rather performs cleanup like closing Statement, ResultSet, and so on created from the logical connection. How to Prevent JDBC Resource Leaks with JDBC and with jOOQ: https://tomcat.apache.org/tomcat-8.5-doc/jdbc-pool.html https://essaytyper.pro https://blog.jooq.org/2017/01/05/how-to-prevent-jdbc-resource-leaks-with-jdbc-and-with-jooq/ -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [VOTE] Release Apache Tomcat Native 1.2.23
Hi all, Just a reminder that this vote is still open. Thanks, Mark On 26/06/2019 14:20, Mark Thomas wrote: > Version 1.2.23 includes the following changes compared to 1.2.21 > > - Fix crashes associated with the use of a CRL > > - Add support for TLS key logging. Patch provided by John Kelly. > > - Make Windows 7 the default build target on Windows > > > Various other fixes and improvements. See the changelog for details. > > The proposed release artefacts can be found at [1], > and the build was done using tag [2]. > > The Apache Tomcat Native 1.2.23 release is > [ ] Stable, go ahead and release > [ ] Broken because of ... > > Thanks, > > Mark > > > [1] > https://dist.apache.org/repos/dist/dev/tomcat/tomcat-connectors/native/1.2.23 > [2] > https://gitbox.apache.org/repos/asf?p=tomcat-native.git;a=commit;h=2f24cb4c2717f170755b74585f33802bf0f371c6 > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: If-Range must use exact match on dates (allowing for resolution)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 92585d37b9b5a2ee5b88c365032db4b6e1be1452
Author: Mark Thomas
AuthorDate: Mon Jul 1 11:27:35 2019 +0100
If-Range must use exact match on dates (allowing for resolution)
---
java/org/apache/catalina/servlets/DefaultServlet.java | 13 +
webapps/docs/changelog.xml| 4
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 261bd58..d5b9ab0 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -1460,22 +1460,19 @@ public class DefaultServlet extends HttpServlet {
long lastModified = resource.getLastModified();
if (headerValueTime == (-1L)) {
-
// If the ETag the client gave does not match the entity
// etag, then the entire entity is returned.
-if (!eTag.equals(headerValue.trim()))
+if (!eTag.equals(headerValue.trim())) {
return FULL;
-
+}
} else {
-
-// If the timestamp of the entity the client got is older than
+// If the timestamp of the entity the client got differs from
// the last modification date of the entity, the entire entity
// is returned.
-if (lastModified > (headerValueTime + 1000))
+if (Math.abs(lastModified -headerValueTime) > 1000) {
return FULL;
-
+}
}
-
}
long fileLength = resource.getContentLength();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f6c6d7b..3c2105c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -55,6 +55,10 @@
be ignored rather than triggering a 416 response. Based on a pull
request by zhanhb. (markt)
+
+When comparing a date from a If-Range header, an exact
+match is required. Based on a pull request by zhanhb. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated (3c1059e -> 92585d3)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git. from 3c1059e Make package versions derive from the upstream projects versions used new f79b542 Ignore Range headers that use unknown units as per RFC 7233. new 92585d3 If-Range must use exact match on dates (allowing for resolution) The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../apache/catalina/servlets/DefaultServlet.java | 51 +- .../servlets/TestDefaultServletRangeRequests.java | 11 +++-- webapps/docs/changelog.xml | 9 3 files changed, 47 insertions(+), 24 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 01/02: Ignore Range headers that use unknown units as per RFC 7233.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f79b5424c498d1cfde1aed6de85285b31484af06
Author: Mark Thomas
AuthorDate: Mon Jul 1 11:17:45 2019 +0100
Ignore Range headers that use unknown units as per RFC 7233.
Based on a pull request by zhanhb.
Also cleaned up the parseRange() return values to differentiate between
"invalid header, send an error response" and "ignore the Range header"
---
.../apache/catalina/servlets/DefaultServlet.java | 38 ++
.../servlets/TestDefaultServletRangeRequests.java | 11 +--
webapps/docs/changelog.xml | 5 +++
3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 4502649..261bd58 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -909,7 +909,7 @@ public class DefaultServlet extends HttpServlet {
}
}
-ArrayList ranges = null;
+ArrayList ranges = FULL;
long contentLength = -1L;
if (resource.isDirectory()) {
@@ -935,6 +935,9 @@ public class DefaultServlet extends HttpServlet {
// Parse range specifier
ranges = parseRange(request, response, resource);
+if (ranges == null) {
+return;
+}
// ETag header
response.setHeader("ETag", eTag);
@@ -1013,12 +1016,7 @@ public class DefaultServlet extends HttpServlet {
conversionRequired = false;
}
-if (resource.isDirectory() ||
-isError ||
-( (ranges == null || ranges.isEmpty())
-&& request.getHeader("Range") == null ) ||
-ranges == FULL ) {
-
+if (resource.isDirectory() || isError || ranges == FULL ) {
// Set the appropriate output headers
if (contentType != null) {
if (debug > 0)
@@ -1438,7 +1436,8 @@ public class DefaultServlet extends HttpServlet {
* @param request The servlet request we are processing
* @param response The servlet response we are creating
* @param resource The resource
- * @return a list of ranges
+ * @return a list of ranges, {@code null} if the range header was invalid
or
+ * {@code #FULL} if the Range header should be ignored.
* @throws IOException an IO error occurred
*/
protected ArrayList parseRange(HttpServletRequest request,
@@ -1482,26 +1481,39 @@ public class DefaultServlet extends HttpServlet {
long fileLength = resource.getContentLength();
if (fileLength == 0) {
-return null;
+// Range header makes no sense for a zero length resource. Tomcat
+// therefore opts to ignore it.
+return FULL;
}
// Retrieving the range header (if any is specified
String rangeHeader = request.getHeader("Range");
if (rangeHeader == null) {
-return null;
+// No Range header is the same as ignoring any Range header
+return FULL;
}
Ranges ranges = Ranges.parseRanges(new StringReader(rangeHeader));
-// bytes is the only range unit supported (and I don't see the point
-// of adding new ones).
-if (ranges == null || !ranges.getUnits().equals("bytes")) {
+if (ranges == null) {
+// The Range header is present but not formatted correctly.
+// Could argue for a 400 response but 416 is more specific.
+// There is also the option to ignore the (invalid) Range header.
+// RFC7233#4.4 notes that many servers do ignore the Range header
in
+// these circumstances but Tomcat has always returned a 416.
response.addHeader("Content-Range", "bytes */" + fileLength);
response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
return null;
}
+// bytes is the only range unit supported (and I don't see the point
+// of adding new ones).
+if (!ranges.getUnits().equals("bytes")) {
+// RFC7233#3.1 Servers must ignore range units they don't
understand
+return FULL;
+}
+
// TODO: Remove the internal representation and use Ranges
// Convert to internal representation
ArrayList result = new ArrayList<>();
diff --git
a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
b/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
index 1f9f189..da5d477 100644
--- a/test/org/apache/catalina/servlets/TestDefaultServletRangeRequests.java
[GitHub] [tomcat] markt-asf commented on issue #175: Apply the suggestion in rfc7233
markt-asf commented on issue #175: Apply the suggestion in rfc7233 URL: https://github.com/apache/tomcat/pull/175#issuecomment-507209534 Thanks. I have applied 2 of the 3 changes to 9.0.x (range units and etag) with some additional refactoring / clean-up as well as additional comments. I thin the remaining change needs to be made optional as many clients have implemented partial PUT even if the intention of the RFC 2616 authors was not to allow it. This is next on my TODO list followed by back-porting all the Rnage improvements inspired by your PRs. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Align with 8.5.x
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0ef94cefaa6ae86791806e83d5f6161786231803
Author: Mark Thomas
AuthorDate: Sun Jun 30 23:53:57 2019 +0100
Align with 8.5.x
---
.../tomcat/websocket/AsyncChannelGroupUtil.java| 2 +-
.../websocket/AsyncChannelWrapperSecure.java | 27 -
.../tomcat/websocket/BackgroundProcessManager.java | 2 +-
java/org/apache/tomcat/websocket/Constants.java| 14 +++--
.../tomcat/websocket/FutureToSendHandler.java | 2 +-
.../apache/tomcat/websocket/PerMessageDeflate.java | 2 +-
.../apache/tomcat/websocket/Transformation.java| 9 +++
.../tomcat/websocket/TransformationFactory.java| 2 +-
java/org/apache/tomcat/websocket/Util.java | 3 +-
java/org/apache/tomcat/websocket/WsFrameBase.java | 70 +-
.../org/apache/tomcat/websocket/WsFrameClient.java | 22 +++
11 files changed, 72 insertions(+), 83 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/AsyncChannelGroupUtil.java
b/java/org/apache/tomcat/websocket/AsyncChannelGroupUtil.java
index 0e81011..fe0136e 100644
--- a/java/org/apache/tomcat/websocket/AsyncChannelGroupUtil.java
+++ b/java/org/apache/tomcat/websocket/AsyncChannelGroupUtil.java
@@ -37,7 +37,7 @@ import org.apache.tomcat.util.threads.ThreadPoolExecutor;
public class AsyncChannelGroupUtil {
private static final StringManager sm =
-StringManager.getManager(Constants.PACKAGE_NAME);
+StringManager.getManager(AsyncChannelGroupUtil.class);
private static AsynchronousChannelGroup group = null;
private static int usageCount = 0;
diff --git a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
index 0f0f038..c5eac9f 100644
--- a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
+++ b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
@@ -51,7 +51,7 @@ public class AsyncChannelWrapperSecure implements
AsyncChannelWrapper {
private final Log log =
LogFactory.getLog(AsyncChannelWrapperSecure.class);
private static final StringManager sm =
-StringManager.getManager(Constants.PACKAGE_NAME);
+StringManager.getManager(AsyncChannelWrapperSecure.class);
private static final ByteBuffer DUMMY = ByteBuffer.allocate(16921);
private final AsynchronousSocketChannel socketChannel;
@@ -268,8 +268,7 @@ public class AsyncChannelWrapperSecure implements
AsyncChannelWrapper {
Future f =
socketChannel.read(socketReadBuffer);
Integer socketRead = f.get();
if (socketRead.intValue() == -1) {
-throw new EOFException(sm.getString(
-"asyncChannelWrapperSecure.eof"));
+throw new
EOFException(sm.getString("asyncChannelWrapperSecure.eof"));
}
}
@@ -277,8 +276,7 @@ public class AsyncChannelWrapperSecure implements
AsyncChannelWrapper {
if (socketReadBuffer.hasRemaining()) {
// Decrypt the data in the buffer
-SSLEngineResult r =
-sslEngine.unwrap(socketReadBuffer, dest);
+SSLEngineResult r = sslEngine.unwrap(socketReadBuffer,
dest);
read += r.bytesProduced();
Status s = r.getStatus();
@@ -405,17 +403,15 @@ public class AsyncChannelWrapperSecure implements
AsyncChannelWrapper {
handshaking = false;
break;
}
-default: {
-throw new SSLException("TODO");
+case NOT_HANDSHAKING: {
+throw new SSLException(
+
sm.getString("asyncChannelWrapperSecure.notHandshaking"));
}
}
}
-} catch (SSLException e) {
-hFuture.fail(e);
-} catch (InterruptedException e) {
-hFuture.fail(e);
-} catch (ExecutionException e) {
+} catch (Exception e) {
hFuture.fail(e);
+return;
}
hFuture.complete(null);
@@ -429,13 +425,14 @@ public class AsyncChannelWrapperSecure implements
AsyncChannelWrapper {
if (resultStatus != Status.OK &&
(wrap || resultStatus != Status.BUFFER_UNDERFLOW)) {
-throw new SSLException("TODO");
+throw new SSLException(
+sm.getString("asyncChannelWrapperSecure.check.notOk",
resultStatus));
[tomcat] 01/02: Align with 8.5.x.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 45170f0138845e609869afb70564b1023ff21711
Author: Mark Thomas
AuthorDate: Sun Jun 30 23:41:22 2019 +0100
Align with 8.5.x.
---
.../apache/tomcat/websocket/server/Constants.java | 7 --
.../server/DefaultServerEndpointConfigurator.java | 6 +++--
.../websocket/server/LocalStrings.properties | 3 +--
.../tomcat/websocket/server/UpgradeUtil.java | 12 +++---
.../tomcat/websocket/server/UriTemplate.java | 3 +--
.../apache/tomcat/websocket/server/WsFilter.java | 2 --
.../tomcat/websocket/server/WsFrameServer.java | 5 -
.../websocket/server/WsHandshakeRequest.java | 4 +---
.../server/WsRemoteEndpointImplServer.java | 4 ++--
java/org/apache/tomcat/websocket/server/WsSci.java | 6 ++---
.../tomcat/websocket/server/WsServerContainer.java | 26 +++---
.../tomcat/websocket/server/WsWriteTimeout.java| 5 +
12 files changed, 38 insertions(+), 45 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/server/Constants.java
b/java/org/apache/tomcat/websocket/server/Constants.java
index 8a86d70..4afb1a7 100644
--- a/java/org/apache/tomcat/websocket/server/Constants.java
+++ b/java/org/apache/tomcat/websocket/server/Constants.java
@@ -21,8 +21,11 @@ package org.apache.tomcat.websocket.server;
*/
public class Constants {
-protected static final String PACKAGE_NAME =
-Constants.class.getPackage().getName();
+/**
+ * @deprecated. Will be removed in 8.5.x onwards.
+ */
+@Deprecated
+protected static final String PACKAGE_NAME =
Constants.class.getPackage().getName();
public static final String BINARY_BUFFER_SIZE_SERVLET_CONTEXT_INIT_PARAM =
"org.apache.tomcat.websocket.binaryBufferSize";
diff --git
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
index 64ac8ca..c6cbbd6 100644
---
a/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
+++
b/java/org/apache/tomcat/websocket/server/DefaultServerEndpointConfigurator.java
@@ -33,8 +33,10 @@ public class DefaultServerEndpointConfigurator
public T getEndpointInstance(Class clazz)
throws InstantiationException {
try {
-return clazz.newInstance();
-} catch (IllegalAccessException e) {
+return clazz.getConstructor().newInstance();
+} catch (InstantiationException e) {
+throw e;
+} catch (ReflectiveOperationException e) {
InstantiationException ie = new InstantiationException();
ie.initCause(e);
throw ie;
diff --git a/java/org/apache/tomcat/websocket/server/LocalStrings.properties
b/java/org/apache/tomcat/websocket/server/LocalStrings.properties
index c987458..85cedd6 100644
--- a/java/org/apache/tomcat/websocket/server/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/server/LocalStrings.properties
@@ -17,9 +17,8 @@ sci.noWebSocketSupport=JSR 356 WebSocket (Java WebSocket 1.1)
support is not ava
serverContainer.addNotAllowed=No further Endpoints may be registered once an
attempt has been made to use one of the previously registered endpoints
serverContainer.configuratorFail=Failed to create configurator of type [{0}]
for POJO of type [{1}]
-serverContainer.duplicatePaths=Multiple Endpoints may not be deployed to the
same path [{0}] : existing endpoint was {1} and new endpoint is {2}
+serverContainer.duplicatePaths=Multiple Endpoints may not be deployed to the
same path [{0}] : existing endpoint was [{1}] and new endpoint is [{2}]
serverContainer.encoderFail=Unable to create encoder of type [{0}]
-serverContainer.endpointDeploy=Endpoint class [{0}] deploying to path [{1}] in
ServletContext [{2}]
serverContainer.failedDeployment=Deployment of WebSocket Endpoints to the web
application with path [{0}] is not permitted due to the failure of a previous
deployment
serverContainer.missingAnnotation=Cannot deploy POJO class [{0}] as it is not
annotated with @ServerEndpoint
serverContainer.missingEndpoint=An Endpoint instance has been request for path
[{0}] but no matching Endpoint class was found
diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
index 7a752ff..36dfbc5 100644
--- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
+++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
@@ -50,8 +50,8 @@ import org.apache.tomcat.websocket.pojo.PojoEndpointServer;
public class UpgradeUtil {
-private static final StringManager sm = StringManager
-
.getManager(org.apache.tomcat.websocket.server.Constants.PACKAGE_NAME);
+private static final
[tomcat] branch 7.0.x updated (bf5e555 -> 0ef94ce)
This is an automated email from the ASF dual-hosted git repository. markt pushed a change to branch 7.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git. from bf5e555 Revert changes that broke compilation with source=1.6 new 45170f0 Align with 8.5.x. new 0ef94ce Align with 8.5.x The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../tomcat/websocket/AsyncChannelGroupUtil.java| 2 +- .../websocket/AsyncChannelWrapperSecure.java | 27 - .../tomcat/websocket/BackgroundProcessManager.java | 2 +- java/org/apache/tomcat/websocket/Constants.java| 14 +++-- .../tomcat/websocket/FutureToSendHandler.java | 2 +- .../apache/tomcat/websocket/PerMessageDeflate.java | 2 +- .../apache/tomcat/websocket/Transformation.java| 9 +++ .../tomcat/websocket/TransformationFactory.java| 2 +- java/org/apache/tomcat/websocket/Util.java | 3 +- java/org/apache/tomcat/websocket/WsFrameBase.java | 70 +- .../org/apache/tomcat/websocket/WsFrameClient.java | 22 +++ .../apache/tomcat/websocket/server/Constants.java | 7 ++- .../server/DefaultServerEndpointConfigurator.java | 6 +- .../websocket/server/LocalStrings.properties | 3 +- .../tomcat/websocket/server/UpgradeUtil.java | 12 +++- .../tomcat/websocket/server/UriTemplate.java | 3 +- .../apache/tomcat/websocket/server/WsFilter.java | 2 - .../tomcat/websocket/server/WsFrameServer.java | 5 +- .../websocket/server/WsHandshakeRequest.java | 4 +- .../server/WsRemoteEndpointImplServer.java | 4 +- java/org/apache/tomcat/websocket/server/WsSci.java | 6 +- .../tomcat/websocket/server/WsServerContainer.java | 26 +++- .../tomcat/websocket/server/WsWriteTimeout.java| 5 +- 23 files changed, 110 insertions(+), 128 deletions(-) - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [GitHub] [tomcat] alpire opened a new pull request #176: CoyoteAdapter: fix out-of-bounds read in checkNormalize
On 29/06/2019 21:01, Mark Thomas wrote: > On 29/06/2019 02:26, GitBox wrote: >> alpire opened a new pull request #176: CoyoteAdapter: fix out-of-bounds read >> in checkNormalize >> URL: https://github.com/apache/tomcat/pull/176 >> >> >>On malformed requests, checkNormalize would throw an >> ArrayIndexOutOfBoundsException leading to a 500 response. This change fixes >> checkNormalize to return false instead of throwing exception on those >> inputs, and adds a few tests to check the new functionality. > > That there are URI sequences that can trigger this is certainly > something that we need to fix. > > On a slightly different topic, this got me thinking.> > checkNormalize() is a test that runs on every request. It is essentially > there to ensure that whatever character set has been specified for the > Connector is "safe". In this case "safe" means when the bytes are > converted to characters we don't get any unexpected "." or "/" > characters in the result that make the character version of the URI > non-normalized. > > Rather than run this test on every request, we could: > - pre-calculate the character sets we consider to be safe > - throw an exception if the user attempts to configure the Connector to > use one > > I think we could safely exclude any character set where any of the > following were not true: > > - 0x00 <-> '\u' > - 0x2e <-> '.' > - 0x2f <-> '/' > - 0x5c <-> '\' > > We could create a unit test that maintains/checks the list of "safe" > character set canonical names. After creating the character set in > Connector.setURIEncoding(), if the canonical name of the resulting > character set is not in the safe list, throw an exception. > > Then remove checkNormalize() and replace with a comment that explains > why the conversion is known to be safe. > > There are several loops through the URI in checkNormalize(). I think - > I'd need to test to confirm - that removing them would provide a > measurable performance benefit. > > Thoughts? I thought about this some more over the weekend and I think it is a good idea in theory but a bad idea in practice. checkNormalize() serves a purpose that wasn't addressed in my comments above. Broken decoders. There is a history of issues with Java's UTF-8 decoder that could be exploited in the way checkNormalize() is designed to protect against. While I am reasonably confident Java's UTF-8 decoder is now safe, I am not confident that every Java decoder is bug free. Therefore, I think the checkNormalize() call needs to be retained for every request. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
