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

get serialNumber from subject #118

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

busla
Copy link

@busla busla commented Sep 6, 2018

Allows the serialNumber to be extracted and validated from cert Subject.

@codecov-io
Copy link

Codecov Report

Merging #118 into master will decrease coverage by 0.1%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
- Coverage   94.89%   94.78%   -0.11%     
==========================================
  Files           3        3              
  Lines         588      595       +7     
==========================================
+ Hits          558      564       +6     
- Misses         30       31       +1
Impacted Files Coverage Δ
signxml/__init__.py 94.48% <90.9%> (-0.15%) ⬇️

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 4e5bf2c...780fcf5. Read the comment docs.

@kislyuk
Copy link
Member

kislyuk commented Nov 2, 2018

Thank you for your contribution. Sorry about the delay in handling this PR.

To proceed, this will require tests and the tightening of the comparisons in _get_serial_number(). The in operator is inappropriate in both comparisons, I think.

(Tests are failing for an unrelated reason - I'll fix that in master.)

def _get_serial_number(self, comps, serial):
for v in comps:
if len(v) == 2 and \
v[0].decode('utf-8') in 'serialNumber' and \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comparison seems wrong. Why are you testing for a substring of 'serialNumber'?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄 definitely some autocomplete/paste issue.

I ended up omitting the serial number validation in my project and left the merge request half done, but since you want to give this attention I'll get back to this.

Thanks for a great library!

@ovnicraft
Copy link

@busla any news about this ? travis is broken

@kislyuk kislyuk force-pushed the develop branch 4 times, most recently from 6b21a86 to 6879c98 Compare November 14, 2022 00:32
@kislyuk kislyuk force-pushed the develop branch 2 times, most recently from 9717621 to 82ae152 Compare November 27, 2022 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants