[WAGON-583] WebDavWagon and tests now adhere to RFC 4918 by slachiewicz · Pull Request #845 · apache/maven-wagon

slachiewicz · 2025-12-30T22:20:41Z

Handle 409 Conflict status when parent collections don't exist
Handle redirects on MKCOL operations
Accept 200 OK status for already-existing collections
Fix tests to use WebDAV PROPFIND instead of filesystem checks

Copilot

Pull request overview

This PR updates the WebDavWagon implementation to comply with RFC 4918 WebDAV specifications, improving the robustness of collection creation operations and refactoring tests to use proper WebDAV PROPFIND requests instead of direct filesystem checks.

Enhances MKCOL operation to handle additional RFC 4918 status codes (200 OK, 409 Conflict)
Adds redirect handling for MKCOL requests
Introduces a new collectionExists method for WebDAV-based collection verification

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
WebDavWagon.java	Added RFC 4918 compliance for status code handling in mkdirs(), implemented redirect handling in doMkCol(), and added collectionExists() method for WebDAV-based collection verification
WebDavWagonTest.java	Refactored testMkdirs() and testMkdirsWithNoBasedir() to use WebDAV PROPFIND via collectionExists() instead of filesystem File.exists() checks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-30T22:23:24Z

+            if (statusCode >= HttpStatus.SC_MULTIPLE_CHOICES && statusCode < HttpStatus.SC_BAD_REQUEST) {
+                org.apache.http.Header locationHeader = closeableHttpResponse.getFirstHeader("Location");
+                if (locationHeader != null) {
+                    String redirectUrl = locationHeader.getValue();
+                    // Recursive call to handle redirect - execute() will handle the redirect automatically
+                    // but we need to return the final status
+                    return doMkCol(redirectUrl);
+                }


The redirect handling logic has a potential infinite recursion issue. If a server responds with a redirect that points back to the original URL or creates a redirect loop, this will cause a stack overflow. The recursive call should include protection against redirect loops, such as tracking visited URLs or limiting the maximum number of redirects.

Copilot · 2025-12-30T22:23:25Z

+            String repositoryUrl = testRepository.getUrl();
+
            // test leading /
-            assertFalse(new File(dir, "foo").exists());
+            String fooUrl = repositoryUrl + (repositoryUrl.endsWith("/") ? "" : "/") + "foo";
+            assertFalse("Collection should not exist before creation", wagon.collectionExists(fooUrl));
            wagon.mkdirs("/foo");
-            assertTrue(new File(dir, "foo").exists());
+            assertTrue("Collection should exist after creation", wagon.collectionExists(fooUrl));

            // test trailing /
-            assertFalse(new File(dir, "bar").exists());
+            String barUrl = repositoryUrl + (repositoryUrl.endsWith("/") ? "" : "/") + "bar";
+            assertFalse("Collection should not exist before creation", wagon.collectionExists(barUrl));
            wagon.mkdirs("bar/");
-            assertTrue(new File(dir, "bar").exists());
+            assertTrue("Collection should exist after creation", wagon.collectionExists(barUrl));

-            // test when already exists
+            // test when already exists (should not fail)
            wagon.mkdirs("bar");
+            assertTrue("Collection should still exist", wagon.collectionExists(barUrl));

            // test several parts
-            assertFalse(new File(dir, "1/2/3/4").exists());
+            String deepUrl = repositoryUrl + (repositoryUrl.endsWith("/") ? "" : "/") + "1/2/3/4";
+            assertFalse("Deep collection should not exist before creation", wagon.collectionExists(deepUrl));
            wagon.mkdirs("1/2/3/4");
-            assertTrue(new File(dir, "1/2/3/4").exists());
+            assertTrue("Deep collection should exist after creation", wagon.collectionExists(deepUrl));

            // test additional part and trailing /
-            assertFalse(new File(dir, "1/2/3/4/5").exists());
+            String deeperUrl = repositoryUrl + (repositoryUrl.endsWith("/") ? "" : "/") + "1/2/3/4/5";
+            assertFalse("Deeper collection should not exist before creation", wagon.collectionExists(deeperUrl));
            wagon.mkdirs("1/2/3/4/5/");
-            assertTrue(new File(dir, "1/2/3/4").exists());
+            assertTrue("Deeper collection should exist after creation", wagon.collectionExists(deeperUrl));


The URL construction pattern is repetitive and error-prone. The same logic for appending paths to repositoryUrl is duplicated multiple times. Consider extracting this into a helper method to reduce duplication and improve maintainability.

- Handle 409 Conflict status when parent collections don't exist - Handle redirects on MKCOL operations - Accept 200 OK status for already-existing collections - Fix tests to use WebDAV PROPFIND instead of filesystem checks

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+            // RFC 4918: Accept 201 Created or 200 OK (if collection already exists from another request)
+            if (status != HttpStatus.SC_CREATED && status != HttpStatus.SC_OK) {


        try (CloseableHttpResponse closeableHttpResponse = execute(method)) {
-            return closeableHttpResponse.getStatusLine().getStatusCode();
+            int statusCode = closeableHttpResponse.getStatusLine().getStatusCode();
+
+            // RFC 4918: Handle redirects for MKCOL
+            // 3xx redirects should be followed to the new location
+            if (statusCode >= HttpStatus.SC_MULTIPLE_CHOICES && statusCode < HttpStatus.SC_BAD_REQUEST) {
+                org.apache.http.Header locationHeader = closeableHttpResponse.getFirstHeader("Location");
+                if (locationHeader != null) {
+                    String redirectUrl = locationHeader.getValue();
+                    // Follow the redirect by recursively invoking doMkCol on the new URL
+                    // and return the status code from the final resolved location
+                    return doMkCol(redirectUrl);
+                }


+            int statusCode = closeableHttpResponse.getStatusLine().getStatusCode();
+
+            // RFC 4918: Handle redirects for MKCOL
+            // 3xx redirects should be followed to the new location
+            if (statusCode >= HttpStatus.SC_MULTIPLE_CHOICES && statusCode < HttpStatus.SC_BAD_REQUEST) {
+                org.apache.http.Header locationHeader = closeableHttpResponse.getFirstHeader("Location");
+                if (locationHeader != null) {
+                    String redirectUrl = locationHeader.getValue();
+                    // Follow the redirect by recursively invoking doMkCol on the new URL
+                    // and return the status code from the final resolved location
+                    return doMkCol(redirectUrl);
+                }
+            }
+
+            return statusCode;


slachiewicz requested a review from Copilot December 30, 2025 22:20

slachiewicz marked this pull request as draft December 30, 2025 22:21

Copilot started reviewing on behalf of slachiewicz December 30, 2025 22:21 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

[WAGON-583] WebDavWagon and tests now adhere to RFC 4918

3f095c3

- Handle 409 Conflict status when parent collections don't exist - Handle redirects on MKCOL operations - Accept 200 OK status for already-existing collections - Fix tests to use WebDAV PROPFIND instead of filesystem checks

slachiewicz force-pushed the vk/03fd-wagon-583-webdav branch from f8c7ea0 to 3f095c3 Compare January 1, 2026 20:58

Apply suggestions from code review

6f5eb51

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

slachiewicz requested a review from Copilot May 10, 2026 14:09

Copilot started reviewing on behalf of slachiewicz May 10, 2026 14:10 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WAGON-583] WebDavWagon and tests now adhere to RFC 4918#845

[WAGON-583] WebDavWagon and tests now adhere to RFC 4918#845
slachiewicz wants to merge 2 commits into
apache:wagon-3.xfrom
slachiewicz:vk/03fd-wagon-583-webdav

slachiewicz commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// RFC 4918: Accept 201 Created or 200 OK (if collection already exists from another request)
		if (status != HttpStatus.SC_CREATED && status != HttpStatus.SC_OK) {

Conversation

slachiewicz commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants