BagLinter always adds 'different_case' warning

I just upgraded to v1.2.0 of this library. With the upgraded stuff on BagIt v1.0 in #118, a new bug has entered the library:

When using the BagLinter, the warning different_case will always be shown. Given it's description:

The bag contains two files that differ only in case. This can cause problems on a filesystem like the one used by apple (HFS).

I would not expect this to happen on a bag with only very distinct files.

I think the bug is as follows: in the code below a manifest file is read and for each line the path is added to the Set paths (after being converted to lowercase).

bagit-java/src/main/java/gov/loc/repository/bagit/conformance/ManifestChecker.java

Lines 119 to 139 in 97a6770

    
           try(final BufferedReader reader = Files.newBufferedReader(manifestFile, encoding)){ 
        
             final Set<String> paths = new HashSet<>(); 
        
             String line = reader.readLine(); 
        
             while(line != null){ 
        
               String path = parsePath(line); 
        
               path = checkForManifestCreatedWithMD5SumTools(path, warnings, warningsToIgnore); 
        
               paths.add(path.toLowerCase()); 
        
               checkForDifferentCase(path, paths, manifestFile, warnings, warningsToIgnore); 
        
               if(encoding.name().startsWith("UTF")){ 
        
                 checkNormalization(path, manifestFile.getParent(), warnings, warningsToIgnore); 
        
               } 
        
               checkForBagWithinBag(line, warnings, warningsToIgnore, isPayloadManifest); 
        
               checkForRelativePaths(line, warnings, warningsToIgnore, manifestFile); 
        
               checkForOSSpecificFiles(line, warnings, warningsToIgnore, manifestFile); 
        
               line = reader.readLine(); 
        
             } 
        
           }

Immediately after that checkForDifferentCase is called, which checks whether paths.contains(path.toLowerCase()). Of course, this is always true, since path was just added to paths before this check.

bagit-java/src/main/java/gov/loc/repository/bagit/conformance/ManifestChecker.java

Lines 174 to 183 in 97a6770

    
             /* 
        
              * Check that the same line doesn't already exist in the set of paths 
        
              */ 
        
             private static void checkForDifferentCase(final String path, final Set<String> paths, final Path manifestFile,  
        
                 final Set<BagitWarning> warnings, final Collection<BagitWarning> warningsToIgnore){ 
        
               if(!warningsToIgnore.contains(BagitWarning.DIFFERENT_CASE) && paths.contains(path.toLowerCase())){ 
        
                 logger.warn(messages.getString("different_case_warning"), manifestFile, path); 
        
                 warnings.add(BagitWarning.DIFFERENT_CASE); 
        
               } 
        
             }

Manifest checker: test for different_case earlier (see LibraryOfCongr…

I created #120 simply to do the check first. Today has been rather busy so a second set of eyes on the code would be appreciated.

@acdha

@acdha From reasoning through the code this looks fine. But I wasn't able to test it with the code change in my own project, since I can't find a way to build this project to my local Maven repository directory. I keep hitting errors with the current gradle configuration.

What surprises me a bit is that no tests we added or changed. I was looking and running ManifestCheckerTest and saw that (even after the change was applied), DIFFERENT_CASE still showed up in the test results. I'm not sure if this is correct or not (you guys know much more about the whole context than me, of course), but it made me wonder whether this change is all there is to it.

@acdha

@acdha I originally created this to catch potential errors like
HASH /data/someFile and HASH /data/SomeFile which on mac is the same file, but on other systems should be two different files. With that in mind, it should be giving a warning that they differ only by case.

After looking at this more, it looks like you are correct and this is indeed from adding it to the paths too early.

refs LibraryOfCongress#119 - fixed different case issue and added tes…

refs LibraryOfCongress#119 - changed sha-1 to sha1 to be more inline …

refs LibraryOfCongress#119 - reverting change to name of message digests

Navigation Menu

Repository navigation

BagLinter always adds 'different_case' warning #119

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	try(final BufferedReader reader = Files.newBufferedReader(manifestFile, encoding)){
	final Set<String> paths = new HashSet<>();

	String line = reader.readLine();
	while(line != null){
	String path = parsePath(line);

	path = checkForManifestCreatedWithMD5SumTools(path, warnings, warningsToIgnore);
	paths.add(path.toLowerCase());

	checkForDifferentCase(path, paths, manifestFile, warnings, warningsToIgnore);
	if(encoding.name().startsWith("UTF")){
	checkNormalization(path, manifestFile.getParent(), warnings, warningsToIgnore);
	}
	checkForBagWithinBag(line, warnings, warningsToIgnore, isPayloadManifest);
	checkForRelativePaths(line, warnings, warningsToIgnore, manifestFile);
	checkForOSSpecificFiles(line, warnings, warningsToIgnore, manifestFile);

	line = reader.readLine();
	}
	}

	/*
	* Check that the same line doesn't already exist in the set of paths
	*/
	private static void checkForDifferentCase(final String path, final Set<String> paths, final Path manifestFile,
	final Set<BagitWarning> warnings, final Collection<BagitWarning> warningsToIgnore){
	if(!warningsToIgnore.contains(BagitWarning.DIFFERENT_CASE) && paths.contains(path.toLowerCase())){
	logger.warn(messages.getString("different_case_warning"), manifestFile, path);
	warnings.add(BagitWarning.DIFFERENT_CASE);
	}
	}

BagLinter always adds 'different_case' warning #119

Description

Activity

acdha commented on Jul 9, 2018

rvanheest commented on Jul 10, 2018

jscancella commented on Jul 12, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions