Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PathMatchingResourcePatternResolver handles path names differently than Spring 5 on Mac OS #29243

Closed
kazuki43zoo opened this issue Oct 2, 2022 · 7 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid

Comments

@kazuki43zoo
Copy link
Contributor

kazuki43zoo commented Oct 2, 2022

I was applied a latest Spring 6 snapshot version, I found the PathMatchingResourcePatternResolver handles path names differently than Spring 5 latest GA version(5.3.23) on Mac OS.
For example, If the file name contains voiced sound characters (e.g. ), the result will be different.

Is this behavior by design or a regression bug as implemented in Spring 6? If this behavior was designed, I will rewrite my library to adjust Spring 6 specification.

Reproducing

Directory structures:

...
└── target
    └── test-classes
        └── org
            └── example
                └── バリューオブジェクト
                    └── ジドウシャ.class

Test Cases:

package org.example;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.core.io.Resource;
import org.springframework.core.io.support.PathMatchingResourcePatternResolver;

import java.io.IOException;
import java.util.stream.Collectors;

class AppTest {
  String path;

  @BeforeEach
  void setup() throws IOException {
    PathMatchingResourcePatternResolver resolver = new PathMatchingResourcePatternResolver();
    Resource resource = resolver.getResources("classpath*:org/example/バリューオブジェクト/**/*.class")[0];
    path = resource.getURL().toString().replaceFirst(".*/test-classes/", "");
    System.out.println(path);
    System.out.println(path.length());
  }

  @Test
  void successRunningOnSpring5() {
    // o  r  g  /  e  x  a  m  p  l  e  /  バ        リ    ュ   ー   オ    ブ        ジ        ェ    ク   ト    /  ジ        ド        ウ    シ   ャ    .  c  l  a  s  s
    // 6f,72,67,2f,65,78,61,6d,70,6c,65,2f,30d0     ,30ea,30e5,30fc,30aa,30d6     ,30b8     ,30a7,30af,30c8,2f,30b8     ,30c9     ,30a6,30b7,30e3,2e,63,6c,61,73,73
    System.out.println(path.chars().mapToObj(Integer::toHexString).collect(Collectors.joining(",")));

    Assertions.assertEquals(34, path.length());
    Assertions.assertEquals("org/example/バリューオブジェクト/ジドウシャ.class", path);
  }

  @Test
  void successRunningOnSpring6() {
    // o  r  g  /  e  x  a  m  p  l  e  /  ハ    ゙  リ    ュ   ー   オ    ブ        シ    ゙   ェ    ク   ト    /  シ   ゙   ト   ゙    ウ    シ   ャ    .  c  l  a  s  s
    // 6f,72,67,2f,65,78,61,6d,70,6c,65,2f,30cf,3099,30ea,30e5,30fc,30aa,30d5,3099,30b7,3099,30a7,30af,30c8,2f,30b7,3099,30c8,3099,30a6,30b7,30e3,2e,63,6c,61,73,73
    System.out.println(path.chars().mapToObj(Integer::toHexString).collect(Collectors.joining(",")));

    Assertions.assertEquals(39, path.length());
    Assertions.assertEquals("org/example/ハ\u3099リューオフ\u3099\u3099ェクト/シ\u3099\u3099ウシャ.class", path);
  }
}

Repro project

https://github.com/kazuki43zoo/repro-spring-project-gh-29243

How to run with Spring 5 GA version:

./mvnw clean test
  • AppTest#successRunningOnSpring5 : success
  • AppTest#successRunningOnSpring6 : fail

How to run with Spring 6 snapshot version:

./mvnw clean test -Pspring6
  • AppTest#successRunningOnSpring5 : fail
  • AppTest#successRunningOnSpring6 : sucess
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 2, 2022
@bclozel bclozel added type: regression A bug that is also a regression in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 2, 2022
@bclozel bclozel added this to the 6.0.0-RC1 milestone Oct 2, 2022
@bclozel
Copy link
Member

bclozel commented Oct 2, 2022

Thanks for reporting this @kazuki43zoo this is probably linked to #29163

@kazuki43zoo
Copy link
Contributor Author

Note: Occurred on Mac OS and Windows OS. Linux OS works fine.

@kazuki43zoo
Copy link
Contributor Author

kazuki43zoo commented Oct 2, 2022

Windows OS is another error. Probably it's same with #29226.

@kazuki43zoo kazuki43zoo changed the title PathMatchingResourcePatternResolver handles path names differently than Spring 5 PathMatchingResourcePatternResolver handles path names differently than Spring 5 on Mac OS Oct 2, 2022
@sbrannen
Copy link
Member

sbrannen commented Oct 4, 2022

Hi @kazuki43zoo,

Thanks for bringing this to our attention.

My initial analysis shows that there are two issues here.

  1. The path is URL-encoded.
  2. The path is not in normalized Unicode form.

You can address # 1 by accessing the URL-decoded path via the URI instead of the URL as follows.

path = resource.getURI().getPath().replaceFirst(".*/test-classes/", "");

As stated in the Javadoc for java.net.URL, you can use java.net.URI to avoid issues with URL-encoding of the path (URI#getPath vs. URI#getRawPath).


You can address # 2 by normalizing the path using java.text.Normalizer as follows.

path = Normalizer.normalize(path, Normalizer.Form.NFC);

Please let us know if those modifications to your test allow the test to pass.

In addition, I will investigate if we can align with the behavior in Spring Framework 5.3.x.

sbrannen added a commit that referenced this issue Oct 4, 2022
This commit introduces tests which serve as "regression tests" for the
behavior of PathMatchingResourcePatternResolver in Spring Framework
5.3.x with regard to URL-encoding and Unicode normalization of resource
paths.

Specifically, the new tests demonstrate that resource paths do NOT need
to be decoded or normalized in 5.3.x.

See gh-29243
marcingrzejszczak pushed a commit to marcingrzejszczak/spring-framework that referenced this issue Oct 4, 2022
This commit introduces tests which serve as "regression tests" for the
behavior of PathMatchingResourcePatternResolver in Spring Framework
5.3.x with regard to URL-encoding and Unicode normalization of resource
paths.

Specifically, the new tests demonstrate that resource paths do NOT need
to be decoded or normalized in 5.3.x.

See spring-projectsgh-29243
marcingrzejszczak pushed a commit to marcingrzejszczak/spring-framework that referenced this issue Oct 4, 2022
@sbrannen
Copy link
Member

sbrannen commented Oct 4, 2022

Team Decision:

After further consideration, we have decided that the use of Resource#getURL and Resource#getURI should be avoided when performing direct comparisons against paths in a file system since those methods make no guarantees about the encoding of the paths with regard to the underlying file system.

Instead, if you wish to perform a direct comparison against a path in a Resource, you should use either the File or Path abstraction.

For example, if you change your original code to the following your tests should pass.

path = resource.getFile().getAbsolutePath().replaceFirst(".*/test-classes/", "");

In light of the above, we are closing this issue.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2022
@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed type: regression A bug that is also a regression labels Oct 4, 2022
@sbrannen sbrannen removed this from the 6.0.0-RC1 milestone Oct 4, 2022
@sbrannen
Copy link
Member

sbrannen commented Oct 4, 2022

As a side note, if you still need to normalize paths you might find that your assertion library provides such support.

For example, AssertJ provides assertThat(path).isEqualToNormalizingUnicode(...).

sbrannen added a commit that referenced this issue Oct 4, 2022
This commit introduces assertions that verify that the sub-path is
properly URL-decoded when a path contains `#`. The additional assertions
are necessary since the existing assertions use Resource#getFilename
which already implicitly decodes the path.

See gh-29243
sbrannen added a commit that referenced this issue Oct 4, 2022
@kazuki43zoo
Copy link
Contributor Author

@sbrannen Thanks for good advice!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants