Maven 2.x Ant Tasks

Default remote repository id not safe

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0.9
  • Fix Version/s: 2.0.10
  • Component/s: dependencies task
  • Labels:
    None
  • Number of attachments :
    0

Description

The default id for a remote repository is just the repo URL. However, a URL typically contains all kind of characters that are not safe for usage in local file paths. E.g. the colon ':' from the URL scheme will just blow up on Windows. The slashes from the URL also cause troubles for a path that is meant to be a simple file name instead of a directory spec.

Better choices for the default repo id could be the host name only or just some hex-encoded MD5-digest of the URL.

Issue Links

Activity

Hide
Paul Gier added a comment - - edited

I'm not sure using the hostname would be a good idea, since not all repository urls have a host name. For example deploying to the local file system "file:///home/user/stuff". Also it seems fairly common that two repositories could be on the same hostname.

Maybe the MD5 digest could work though.

When would the repository id be used in a file path that the special characters would blow up?

Show
Paul Gier added a comment - - edited I'm not sure using the hostname would be a good idea, since not all repository urls have a host name. For example deploying to the local file system "file:///home/user/stuff". Also it seems fairly common that two repositories could be on the same hostname. Maybe the MD5 digest could work though. When would the repository id be used in a file path that the special characters would blow up?
Hide
Benjamin Bentmann added a comment -

[...] since not all repository urls have a host name

Would be "localhost" then, isn't it? Anyway, this was just a half-thought idea.

When would the repository id be used in a file path that the special characters would blow up?

Oh yes, I forgot to mention that. The repo id will be used for the metadata files, i.e. maven-metadata-<repo-id>.xml.

Show
Benjamin Bentmann added a comment -
[...] since not all repository urls have a host name
Would be "localhost" then, isn't it? Anyway, this was just a half-thought idea.
When would the repository id be used in a file path that the special characters would blow up?
Oh yes, I forgot to mention that. The repo id will be used for the metadata files, i.e. maven-metadata-<repo-id>.xml.
Hide
Herve Boutemy added a comment -

I updated in r770933 the unit test for MANTTASKS-103 to show that DeployTask.java "automagically" replaces repo id with "remote" to avoid the problem reported here

can you check if a problem persists?

Show
Herve Boutemy added a comment - I updated in r770933 the unit test for MANTTASKS-103 to show that DeployTask.java "automagically" replaces repo id with "remote" to avoid the problem reported here can you check if a problem persists?
Hide
Benjamin Bentmann added a comment -

The issue has a broader scope than just deployment, it also affects artifact resolution/download. Eventually, it seems that RemoteRepository.getDefaultId() needs to be fixed.

Show
Benjamin Bentmann added a comment - The issue has a broader scope than just deployment, it also affects artifact resolution/download. Eventually, it seems that RemoteRepository.getDefaultId() needs to be fixed.
Hide
Paul Gier added a comment -

[...] since not all repository urls have a host name

Would be "localhost" then, isn't it? Anyway, this was just a half-thought idea.

Not necessarily. For "file:///home/user/stuff", java.net.URL just returns an empty string for the hostname. But I guess it depends on what you use to parse the URL.
But I think the hostname is not good to use anyway because of the possible duplicates.

Show
Paul Gier added a comment -
[...] since not all repository urls have a host name Would be "localhost" then, isn't it? Anyway, this was just a half-thought idea.
Not necessarily. For "file:///home/user/stuff", java.net.URL just returns an empty string for the hostname. But I guess it depends on what you use to parse the URL. But I think the hostname is not good to use anyway because of the possible duplicates.
Hide
Herve Boutemy added a comment -

ok, you're right Benjamin (how could I ever imagine the contrary... )

ant -f sample.build.xml test-deps-two-repos

on my Linux box, I get a weird target/tmp/it/ant-tasks/snapshotUniqueFalse/2.0.7-SNAPSHOT/maven-metadata-file: directory
I suppose it simply fails on a Windows box...

Show
Herve Boutemy added a comment - ok, you're right Benjamin (how could I ever imagine the contrary... )
ant -f sample.build.xml test-deps-two-repos
on my Linux box, I get a weird target/tmp/it/ant-tasks/snapshotUniqueFalse/2.0.7-SNAPSHOT/maven-metadata-file: directory I suppose it simply fails on a Windows box...
Hide
Herve Boutemy added a comment -

I just checked: as expected, Maven core does not support remote repositories without id

org.apache.maven.artifact.InvalidRepositoryException: Repository ID must not be empty (URL is: file://${user.dir}/src/test/repo).
        at org.apache.maven.project.ProjectUtils.buildArtifactRepository(ProjectUtils.java:101)

Then I think Maven Ant Tasks should not really support this case either: any algorithm that tries to compute a human readable defautl id cannot work.

I propose the following:

  • add a warning if no id is defined
  • compute a default value as simple as hex-encoded MD5-digest
Show
Herve Boutemy added a comment - I just checked: as expected, Maven core does not support remote repositories without id
org.apache.maven.artifact.InvalidRepositoryException: Repository ID must not be empty (URL is: file://${user.dir}/src/test/repo).
        at org.apache.maven.project.ProjectUtils.buildArtifactRepository(ProjectUtils.java:101)
Then I think Maven Ant Tasks should not really support this case either: any algorithm that tries to compute a human readable defautl id cannot work. I propose the following:
  • add a warning if no id is defined
  • compute a default value as simple as hex-encoded MD5-digest
Hide
Benjamin Bentmann added a comment -

you're right Benjamin (how could I ever imagine the contrary..

Benjamin IS_A human, human IS_A error-prone component

I propose the following: [...]

+1

Show
Benjamin Bentmann added a comment -
you're right Benjamin (how could I ever imagine the contrary..
Benjamin IS_A human, human IS_A error-prone component
I propose the following: [...]
+1
Hide
Paul Gier added a comment -

Fixed in r771907

Show
Paul Gier added a comment - Fixed in r771907

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: