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.

Sign up for GitHub

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

Jump to bottom

withFileFromClassPath fails when path to classpath resource contains spaces #263

Closed
lbialy opened this issue Jan 13, 2017 · 6 comments
Closed

withFileFromClassPath fails when path to classpath resource contains spaces #263

lbialy opened this issue Jan 13, 2017 · 6 comments
Assignees
@rnorth
Milestone
1.3.0

Comments

@lbialy
Copy link

lbialy commented Jan 13, 2017

Hello,

How to replicate:

  1. create directory with spaces in path
mkdir ~/dir\ with\ spaces
  1. move some project using testcontainers-java to that directory
  2. put a file to copy into container in resources directory (touch src/test/resources/any_file in this example)
  3. use container loading files from class path, ie:
new GenericContainer(
        new ImageFromDockerfile()
                .withDockerfileFromBuilder(builder -> {
                        builder
                                .from("alpine:3.2")
                                .copy("any_file", "any_file")
                                .build(); 
                        })
                )
                .withFileFromClasspath("any_file", "any_file")
)

This will throw exception:

1 error
Caused by: org.testcontainers.containers.ContainerFetchException: Can't get Docker image name from org.testcontainers.images.builder.ImageFromDockerfile@61ae0d43
Caused by: java.lang.RuntimeException: Can't get size from ~/dir%20with%20spaces/your-project/target/test-classes/any_file
Caused by: java.nio.file.NoSuchFileException: ~/dir%20with%20spaces/your-project/target/test-classes/any_file

Issue is related to urlencoded spaces in path (%20) when using

URL resource = ClasspathTrait.class.getClassLoader().getResource(resourcePath);
String resourceFilePath = new File(resource.getFile()).getAbsolutePath();

in default implementation of withFileFromClasspath. Calling .toURI() instead of .getFile() on resource should fix this issue while not breaking anything - I'm using this workaround and seems to be fine. No PR though, sorry, simply no time to do it.
@rnorth
Copy link
Member

rnorth commented Jan 14, 2017

Hi @lbialy
Thanks for reporting, and sorry for the problem and thorough analysis. We'll try and address soon.
Richard

@rnorth rnorth self-assigned this Jan 14, 2017
rnorth added a commit that referenced this issue Jan 15, 2017
@rnorth
Ensure that piped streams are closed automatically. This resolves an …
52a7250 
…apparently deadlock somewhere between netty/docker if a previous test, affected by #263, has failed during copying of a file.
@lbialy
Copy link
Author

lbialy commented Jan 17, 2017

@rnorth I think you wanted to mention #262, not #263 here. Glad to see something found.

rnorth added a commit that referenced this issue Jan 21, 2017
@rnorth
Ensure that piped streams are closed automatically. This resolves an …
dc1b260 
…apparently deadlock somewhere between netty/docker if a previous test, affected by #263, has failed during copying of a file.
@rnorth rnorth mentioned this issue Jan 21, 2017
Merged
rnorth added a commit that referenced this issue Jan 22, 2017
@rnorth
Introduce an abstraction over files and classpath resources.
a819a30 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Additional tests for the specific problem in #263 will be added shortly.
@rnorth rnorth added this to the 1.1.8 milestone Jan 22, 2017
@rnorth rnorth mentioned this issue Jan 22, 2017
Merged
@rnorth rnorth modified the milestones: 1.1.9, 1.1.8 Jan 22, 2017
@rnorth rnorth modified the milestones: 1.1.9, 1.1.10 Feb 12, 2017
rnorth added a commit that referenced this issue Mar 12, 2017
@rnorth
Introduce an abstraction over files and classpath resources.
abfe039 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Additional tests for the specific problem in #263 will be added shortly.
rnorth added a commit that referenced this issue Mar 12, 2017
@rnorth
Introduce an abstraction over files and classpath resources.
19905c0 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Additional tests for the specific problem in #263 will be added shortly.
rnorth added a commit that referenced this issue Mar 12, 2017
@rnorth
Update CHANGELOG for 1.2.0 release:
1ef0193 
## [1.2.0] - 2017-03-12
### Fixed
- Fix various escaping issues that may arise when paths contain spaces (#263, #279)
- General documentation fixes/improvements (#300, #303, #304)
- Improve reliability of `ResourceReaper` when there are a large number of containers returned by `docker ps -a` (#295)

### Changed
- Support Docker for Windows via TCP socket connection (#291, #297, #309). _Note that Docker Compose is not yet supported under Docker for Windows (see #306)
- Expose `docker-java`'s `CreateContainerCmd` API for low-level container tweaking (#301)
- Shade `org.newsclub` and Guava dependencies (#299, #292)
- Add `org.testcontainers` label to all containers created by Testcontainers (#294)
@rnorth
Copy link
Member

rnorth commented Mar 13, 2017

@lbialy this is hopefully fixed now - please could you let me know if you encounter any more issues?

I've been developing under a path containing space for a while, just to make sure 😃

rnorth added a commit that referenced this issue Apr 10, 2017
@rnorth
Introduce an abstraction over files and classpath resources.
5a39311 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Additional tests for the specific problem in #263 will be added shortly.
rnorth added a commit that referenced this issue Apr 14, 2017
@rnorth
Introduce an abstraction over files and classpath resources.
6351b1f 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Use recursive copy for docker context TAR to allow contents of directories to be used in Dockerfiles and Docker Compose contexts
rnorth added a commit that referenced this issue Apr 14, 2017
@rnorth
Introduce an abstraction over files and classpath resources. (#313)
7642f98 
This encapsulates all the complexity of generating a path that the Docker daemon is about to create a volume mount for.

This should resolve a general problem with spaces in paths, which was seen in one particular form as #263.

Use recursive copy for docker context TAR to allow contents of directories to be used in Dockerfiles and Docker Compose contexts
@bsideup bsideup modified the milestones: 1.3.0, 1.2.0 May 29, 2017
@bsideup
Copy link
Member

bsideup commented Jun 10, 2017

Hi @lbialy
We released 1.3.0 with a few fixes to file mappings, could you please try it?
Thanks!

@lbialy
Copy link
Author

lbialy commented Jun 14, 2017

I'll try, thanks!

@bsideup
Copy link
Member

bsideup commented Apr 11, 2018

Fixed by #279.

@bsideup bsideup closed this as completed Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rnorth rnorth

Labels
None yet
Projects
None yet
Milestone
1.3.0
Development

No branches or pull requests
3 participants
@rnorth @bsideup @lbialy