Maven 2 & 3

checksum comparison should be case-insensitive

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0.4
  • Fix Version/s: 2.0.9
  • Component/s: Artifacts and Repositories
  • Labels:
    None
  • Complexity:
    Intermediate
  • Patch Submitted:
    Yes
  • Number of attachments :
    4

Description

Validation of MD5 and SHA1 checksums should be case-insensitive (since the individual characters represent hexadecimal digits). For example:

[WARNING] *** CHECKSUM FAILED - Checksum failed on download: local = '9657ed010cea89caa1e35ae326dfccf28ea007f5'; remote
= '9657ED010CEA89CAA1E35AE326DFCCF28EA007F5' - RETRYING

Attachments

  1. Text File
    case-insensitive-checksums-2.0.x.patch
    08/Mar/08 5:13 AM
    0.8 kB
    Benjamin Bentmann
  2. Text File
    case-insensitive-checksums-2.1.x.patch
    10/Mar/08 7:56 PM
    15 kB
    Benjamin Bentmann
  3. Text File
    case-insensitive-checksums-2.1.x.patch
    08/Mar/08 5:13 AM
    0.7 kB
    Benjamin Bentmann
  4. Text File
    mng-2744-it.patch
    11/Mar/08 6:02 PM
    10 kB
    Benjamin Bentmann

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Bentmann added a comment -

Patches against 2.0.x branch (i.e. for maven-artifact-manager) and trunk (i.e. maven-artifact:3.0).

Show
Benjamin Bentmann added a comment - Patches against 2.0.x branch (i.e. for maven-artifact-manager) and trunk (i.e. maven-artifact:3.0).
Hide
Permalink
Benjamin Bentmann added a comment -

Rescheduled for 2.0.9 since it seems to be a one-liner.

Show
Benjamin Bentmann added a comment - Rescheduled for 2.0.9 since it seems to be a one-liner.
Hide
Permalink
Brian Fox added a comment -

Ben, we need tests for these things so it's not just a one liner. A unit test at minimum, an IT test in addition would be better.

Show
Brian Fox added a comment - Ben, we need tests for these things so it's not just a one liner. A unit test at minimum, an IT test in addition would be better.
Hide
Permalink
Benjamin Bentmann added a comment - - edited

Brian, I don't want to question your endeavor to increase test coverage but I don't agree that this is a reason to prevent trivial fixes with tiny code changes from being applied as soon as possible. I would rather suggest to create sub tasks "Add test for XYZ" for those trivial patches where an accompanying test is nice to have but not necessary.

Anyway, here's my next try to patch the trunk, was really more than one line I's not elegant but shows the failure if unpatched.

Show
Benjamin Bentmann added a comment - - edited Brian, I don't want to question your endeavor to increase test coverage but I don't agree that this is a reason to prevent trivial fixes with tiny code changes from being applied as soon as possible. I would rather suggest to create sub tasks "Add test for XYZ" for those trivial patches where an accompanying test is nice to have but not necessary. Anyway, here's my next try to patch the trunk, was really more than one line I's not elegant but shows the failure if unpatched.
Hide
Permalink
Brian Fox added a comment -

I never said I wouldn't fix it, I was just commenting that we need to consider testing when judging when something gets fixed. This functionality is been this way since 2.0 and with only two votes, it's not exactly screaming to be fixed now...however trivial it is.

This particular issue happens to be in maven artifact, which i know has already been branched and is being rewritten. I can take your patch & unit test and apply it to the trunk, but without an IT that lives outside the code, there is no guarantee this won't come back in a month. The ITs are what we need to ensure that the refactoring doesn't overlook existing behavior.

I'm hoping that your motivation now to get this in will get an IT test so I don't have to make a subtask that says "add test for..." because we all know that no one will ever come along and fix that issue. Besides, it's a lot easier to test the IT when you have handy the code that just fixed it so you can test that it fails appropriately.

So there, I tipped my hand, I'm going to apply this no matter what you do, would you still make an IT for it? Pretty please?

Show
Brian Fox added a comment - I never said I wouldn't fix it, I was just commenting that we need to consider testing when judging when something gets fixed. This functionality is been this way since 2.0 and with only two votes, it's not exactly screaming to be fixed now...however trivial it is. This particular issue happens to be in maven artifact, which i know has already been branched and is being rewritten. I can take your patch & unit test and apply it to the trunk, but without an IT that lives outside the code, there is no guarantee this won't come back in a month. The ITs are what we need to ensure that the refactoring doesn't overlook existing behavior. I'm hoping that your motivation now to get this in will get an IT test so I don't have to make a subtask that says "add test for..." because we all know that no one will ever come along and fix that issue. Besides, it's a lot easier to test the IT when you have handy the code that just fixed it so you can test that it fails appropriately. So there, I tipped my hand, I'm going to apply this no matter what you do, would you still make an IT for it? Pretty please?
Hide
Permalink
Benjamin Bentmann added a comment -

we all know that no one will ever come along and fix that issue

...

would you still make an IT for it? Pretty please?

Could I have said "no" to a kind guy ? OK, so here's my first try at a core-it, seems to happily fail right now.

Show
Benjamin Bentmann added a comment -
we all know that no one will ever come along and fix that issue
...
would you still make an IT for it? Pretty please?
Could I have said "no" to a kind guy ? OK, so here's my first try at a core-it, seems to happily fail right now.
Hide
Permalink
Benjamin Bentmann added a comment -

I forgot to mention: During creation of the IT I noticed that the build will pass if only the b.pom.sha1 uses the upper-case checksum (i.e. b.jar can be successfully verified), Maven 2.0.8 merely prints a warning about the checksum verification although the checksum policies in the POM are set to "fail". Is this intended behavior or something to create another issue for?

Show
Benjamin Bentmann added a comment - I forgot to mention: During creation of the IT I noticed that the build will pass if only the b.pom.sha1 uses the upper-case checksum (i.e. b.jar can be successfully verified), Maven 2.0.8 merely prints a warning about the checksum verification although the checksum policies in the POM are set to "fail". Is this intended behavior or something to create another issue for?
Hide
Permalink
Brian Fox added a comment -

applied, thanks for the patch and super-thanks for the IT.

Show
Brian Fox added a comment - applied, thanks for the patch and super-thanks for the IT.

People

Vote (2)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.