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

searching for the public key is now case insensitive. #6

Closed
wants to merge 1 commit into from
Closed

searching for the public key is now case insensitive. #6

wants to merge 1 commit into from

Conversation

mgrossmann
Copy link

@mgrossmann mgrossmann commented Jul 28, 2017

it would be nice to ignore the case while searching for keyrings

ps. my code changes belong to the same license like the original code.

@codecov
Copy link

codecov bot commented Jul 28, 2017

Codecov Report

Merging #6 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master      #6   +/-   ##
========================================
  Coverage      78.9%   78.9%           
  Complexity      173     173           
========================================
  Files            32      32           
  Lines           782     782           
  Branches        104     104           
========================================
  Hits            617     617           
  Misses          105     105           
  Partials         60      60
Impacted Files Coverage Δ Complexity Δ
bouncycastle/openpgp/keys/PGPUtilities.java 83.72% <0%> (ø) 21% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a941b03...fa2adec. Read the comment docs.

@neuhalje
Copy link
Owner

Hi Mike,

thanks for the PR!

I tested with several versions of gpg2 and cases insensitive is indeed the default behaviour.

Two minor issues:

  • Please also make sure that extractSecretSigningKeyFromKeyrings and extractPublicKeyRingForUserId behave alike (your patch introduces a diverging behaviour).
  • Would you mind to add matching unit tests?

@mgrossmann
Copy link
Author

Hi Jens,

i will fix both isues.

/mig

@neuhalje
Copy link
Owner

ping :-)

@mgrossmann
Copy link
Author

I am currently on vacation. Then I will take care of the case.

@neuhalje
Copy link
Owner

Enjoy your vacation! I did some (minor) refactorings, please don't forget to pull before continuing on the code

@mgrossmann
Copy link
Author

I'm still alive. :) I have fixed your first request. I will try to implement the tests this week.

@neuhalje
Copy link
Owner

neuhalje commented Sep 26, 2017

Hi @mgrossmann,

my reformatting of the source code made this merge very, very messy.

The easiest way would be for you to check out the current master (warning, this will force the current branch to my master, removing all your changes ) :

git fetch https://github.com/mgrossmann/bouncy-gpg.git master
git reset --hard FETCH_HEAD

and manually re-apply your commit 477c7c0:

--- a/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/PGPUtilities.java
+++ b/src/main/java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/PGPUtilities.java
@@ -80,8 +80,9 @@ public final class PGPUtilities {
             throw new NullPointerException("publicKeyRings must not be null");
         }

-        // the true parameter indicates, that partial matching of the publicKeyUid is enough.
-        final Iterator<?> keyRings = publicKeyRings.getKeyRings("<" + publicKeyUid + ">", true);
+        // the first true parameter indicates, that partial matching of the publicKeyUid is enough.
+        // the second true parameter indicates, that matching the publicKeyUid is case insensitive.
+        final Iterator<?> keyRings = publicKeyRings.getKeyRings("<" + publicKeyUid + ">", true, true);
         PGPPublicKeyRing returnKeyRing = null;
         while (keyRings.hasNext()) {
             final Object currentKeyRing = keyRings.next();

Sorry for the inconvenience

@neuhalje
Copy link
Owner

neuhalje commented Jan 2, 2018

Hi, any progress on the test? Cheers!

@mgrossmann
Copy link
Author

mgrossmann commented Jan 4, 2018 via email

@neuhalje
Copy link
Owner

Implemented in Rfc4880KeySelectionStrategy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants