This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 459934b0320beb0de6ab5c9cf019f9feed9c3fef Author: opencode <[email protected]> AuthorDate: Wed Oct 14 18:59:19 2026 +0200 Reject If headers containing an empty list in the WebDAV servlet RFC 4918 section 10.4.2 requires a WebDAV If header list to contain at least one condition (List = "(" 1*Condition ")"). The parser in WebdavIfHeader previously accepted an empty list, such as "If: ()", "( )" or a list without any valid entry, and returned a non-null structure for it. An empty IfList.match() vacuously returned true, so the header passed the If precondition checks for any token and ETag, and hasValue() returned true so the strict mode rejection of invalid If syntax in WebdavServlet.checkIfHeaders() was bypassed. parseIfList() now returns null when it did not read at least one valid entry, and parseUntagged() and parseTagged() propagate this so the whole header value is treated as invalid. This routes processing into the existing invalid syntax handling: with strictIfProcessing enabled, the default, requests using such headers now fail the precondition check; with strictIfProcessing disabled the header is ignored, as for any other invalid If header syntax, and lock tokens in the raw header are still evaluated separately by isLocked(). Lenient error recovery for entries that fail to parse is retained where other valid entries were read. Add TestWebdavIfHeader covering the parser behavior and new TestWebdavServlet cases covering the strict and non-strict servlet behavior for "If: ()". --- .../apache/tomcat/util/http/WebdavIfHeader.java | 43 +++++-- .../catalina/servlets/TestWebdavServlet.java | 62 ++++++++++ .../tomcat/util/http/TestWebdavIfHeader.java | 134 +++++++++++++++++++++ 3 files changed, 231 insertions(+), 8 deletions(-) diff --git a/java/org/apache/tomcat/util/http/WebdavIfHeader.java b/java/org/apache/tomcat/util/http/WebdavIfHeader.java index 728a6c4ea0..3c72889122 100644 --- a/java/org/apache/tomcat/util/http/WebdavIfHeader.java +++ b/java/org/apache/tomcat/util/http/WebdavIfHeader.java @@ -154,9 +154,10 @@ public class WebdavIfHeader { } /** - * Returns true if an If header is present. False otherwise. + * Returns true if an If header is present and was parsed successfully. False if no If header is present or the + * header value contains invalid syntax. * - * @return true if an If header is present. + * @return true if an If header is present and was parsed successfully. False otherwise. */ public boolean hasValue() { return ifHeader != null; @@ -268,7 +269,7 @@ public class WebdavIfHeader { * * @param reader the reader * - * @return the parsed map + * @return the parsed map, or <code>null</code> if an invalid (empty) <em>IfList</em> was encountered */ private IfHeaderMap parseTagged(StringReader reader) throws IOException { IfHeaderMap map = new IfHeaderMap(); @@ -283,7 +284,13 @@ public class WebdavIfHeader { String resource = readWord(reader, '>'); if (resource != null) { // go to untagged after reading the resource - map.put(resource, parseUntagged(reader)); + IfHeaderList untagged = parseUntagged(reader); + if (untagged == null) { + // An invalid (empty) IfList makes the whole header + // invalid, stop parsing + return null; + } + map.put(resource, untagged); resources.add(resource); } else { break; @@ -308,7 +315,8 @@ public class WebdavIfHeader { * * @param reader The <code>StringReader</code> to read from for parsing * - * @return An <code>ArrayList</code> of {@link IfList} entries. + * @return An <code>ArrayList</code> of {@link IfList} entries, or <code>null</code> if an invalid (empty) + * <em>IfList</em> was encountered. */ private IfHeaderList parseUntagged(StringReader reader) throws IOException { IfHeaderList list = new IfHeaderList(); @@ -322,7 +330,13 @@ public class WebdavIfHeader { } else if (c == '(') { // start of an IfList, parse - list.add(parseIfList(reader)); + IfList ifList = parseIfList(reader); + if (ifList == null) { + // An invalid (empty) IfList makes the whole header + // invalid, stop parsing + return null; + } + list.add(ifList); } else if (c == '<') { // start of a tag, return current list @@ -348,7 +362,8 @@ public class WebdavIfHeader { * * @param reader The <code>StringReader</code> to read from for parsing * - * @return The {@link IfList} for the input <em>IfList</em>. + * @return The {@link IfList} for the input <em>IfList</em>, or <code>null</code> if the list did not contain at + * least one valid entry. * * @throws IOException if a problem occurs during reading. */ @@ -426,7 +441,19 @@ public class WebdavIfHeader { } } - // return the current list anyway + // RFC 4918 section 10.4.2 requires an IfList to contain at least one + // condition: + // List = "(" 1*Condition ")" + // An empty list (e.g. "()") is invalid syntax. Return null so that + // the caller treats the entire header value as invalid which allows + // strict mode to reject it. + if (res.isEmpty()) { + if (log.isTraceEnabled()) { + log.trace("parseIfList: Empty If list, treating header as invalid"); + } + return null; + } + return res; } diff --git a/test/org/apache/catalina/servlets/TestWebdavServlet.java b/test/org/apache/catalina/servlets/TestWebdavServlet.java index 70530eb57c..9d1116b7f6 100644 --- a/test/org/apache/catalina/servlets/TestWebdavServlet.java +++ b/test/org/apache/catalina/servlets/TestWebdavServlet.java @@ -1537,6 +1537,68 @@ public class TestWebdavServlet extends TomcatBaseTest { Assert.assertEquals(WebdavStatus.SC_CREATED, client.getStatusCode()); } + /* + * An If header value that contains an empty list is invalid syntax (RFC 4918 requires at least one entry in a + * list). With strict If processing (the default) such a header must fail the precondition check. Without strict + * If processing the header is ignored, as for any other invalid If header syntax. + */ + @Test + public void testIfHeaderEmptyListStrict() throws Exception { + Client client = doTestIfHeaderEmptyList("webdav-emptyif-strict", true); + client.setRequest(new String[] { + "PUT /file1.txt HTTP/1.1" + CRLF + + "Host: localhost:" + getPort() + CRLF + + "If: ()" + CRLF + + "Content-Length: 6" + CRLF + + "Connection: Close" + CRLF + + CRLF + + CONTENT + }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_PRECONDITION_FAILED, client.getStatusCode()); + } + + + @Test + public void testIfHeaderEmptyListNonStrict() throws Exception { + Client client = doTestIfHeaderEmptyList("webdav-emptyif-nonstrict", false); + client.setRequest(new String[] { + "PUT /file1.txt HTTP/1.1" + CRLF + + "Host: localhost:" + getPort() + CRLF + + "If: ()" + CRLF + + "Content-Length: 6" + CRLF + + "Connection: Close" + CRLF + + CRLF + + CONTENT + }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + } + + + private Client doTestIfHeaderEmptyList(String webappName, boolean strictIfProcessing) throws Exception { + Tomcat tomcat = getTomcatInstance(); + + // Create a temp webapp that can be safely written to + File tempWebapp = new File(getTemporaryDirectory(), webappName); + tempWebapp.deleteOnExit(); + Assert.assertTrue(tempWebapp.mkdirs()); + Context ctxt = tomcat.addContext("", tempWebapp.getAbsolutePath()); + Wrapper webdavServlet = Tomcat.addServlet(ctxt, "webdav", new WebdavServlet()); + webdavServlet.addInitParameter("readonly", "false"); + webdavServlet.addInitParameter("strictIfProcessing", Boolean.toString(strictIfProcessing)); + ctxt.addServletMapping("/*", "webdav"); + ctxt.addMimeMapping("txt", "text/plain"); + tomcat.start(); + + Client client = new Client(); + client.setPort(getPort()); + return client; + } + + @Test public void testPropertyStore() throws Exception { Tomcat tomcat = getTomcatInstance(); diff --git a/test/org/apache/tomcat/util/http/TestWebdavIfHeader.java b/test/org/apache/tomcat/util/http/TestWebdavIfHeader.java new file mode 100644 index 0000000000..d9b556919b --- /dev/null +++ b/test/org/apache/tomcat/util/http/TestWebdavIfHeader.java @@ -0,0 +1,134 @@ +/* + * 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. + */ +package org.apache.tomcat.util.http; + +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +public class TestWebdavIfHeader { + + private static final String TOKEN = "urn:uuid:0123456789abcdef"; + private static final List<String> TOKENS = Collections.singletonList(TOKEN); + + private static final String URI_PREFIX = "http://localhost/webdav/";; + + /* + * RFC 4918 section 10.4.2 requires an IfList to contain at least one condition: + * + * List = "(" 1*Condition ")" + * + * An empty list must make the header value invalid (hasValue() returns false) so that strict mode processing + * rejects it rather than matching every token and etag vacuously. + */ + @Test + public void testEmptyIfListIsInvalid() throws Exception { + assertInvalid("()"); + assertInvalid("( )"); + assertInvalid("(Not)"); + assertInvalid("(Not )"); + assertInvalid("(junk)"); + assertInvalid("(<urn"); + // A single empty list invalidates the entire header value + assertInvalid("(<" + TOKEN + ">) ()"); + assertInvalid("() (<" + TOKEN + ">)"); + assertInvalid("</myfolder/> ()"); + } + + @Test + public void testAbsentHeaderHasNoValue() throws Exception { + assertInvalid(null); + assertInvalid(""); + } + + @Test + public void testUntaggedMatches() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + ">)"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + Assert.assertFalse(header.matches("/myfolder/", Collections.singletonList("other"), "etag")); + Assert.assertTrue(header.matches("/other/", TOKENS, "etag")); + } + + @Test + public void testEtagMatches() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "([etag])"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + Assert.assertFalse(header.matches("/myfolder/", TOKENS, "other")); + } + + @Test + public void testMultipleLists() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + ">) ([etag])"); + Assert.assertTrue(header.hasValue()); + // Untagged lists are alternatives: the first list matches the token + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "other")); + // The second list matches the etag + Assert.assertTrue(header.matches("/myfolder/", Collections.singletonList("other"), "etag")); + } + + @Test + public void testTaggedMatches() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "</myfolder/> (<" + TOKEN + ">)"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + Assert.assertFalse(header.matches("/myfolder/", Collections.singletonList("other"), "etag")); + // A resource not listed in a tagged header does not match + Assert.assertFalse(header.matches("/other/", TOKENS, "etag")); + } + + @Test + public void testNotMatches() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "(Not <" + TOKEN + ">)"); + Assert.assertTrue(header.hasValue()); + Assert.assertFalse(header.matches("/myfolder/", TOKENS, "etag")); + Assert.assertTrue(header.matches("/myfolder/", Collections.singletonList("other"), "etag")); + } + + /* + * The parser is deliberately lenient with malformed entries that do not prevent the rest of the header from + * parsing (error recovery inherited from the Jackrabbit code). Only a list without any valid entry invalidates + * the header value. + */ + @Test + public void testErrorRecoveryKeepsValidEntries() throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + "> junk)"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + + header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + ">) junk ([etag])"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "other")); + Assert.assertTrue(header.matches("/myfolder/", Collections.singletonList("other"), "etag")); + + header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + "> <incomplete"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + + header = new WebdavIfHeader(URI_PREFIX, "(<" + TOKEN + ">)]"); + Assert.assertTrue(header.hasValue()); + Assert.assertTrue(header.matches("/myfolder/", TOKENS, "etag")); + } + + private void assertInvalid(String headerValue) throws Exception { + WebdavIfHeader header = new WebdavIfHeader(URI_PREFIX, headerValue); + Assert.assertFalse("Expected invalid header for value: " + headerValue, header.hasValue()); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
