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.

Sign up for GitHub

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

Jump to bottom

asn1: handle GENERALIZEDTIME without seconds #88

Merged
merged 2 commits into from
Dec 10, 2016
Merged

asn1: handle GENERALIZEDTIME without seconds #88

merged 2 commits into from
Dec 10, 2016

Conversation

yogo1212
Copy link

@yogo1212 yogo1212 commented Dec 6, 2016 •
edited
Loading

encountered this error using a certificate that is accepted by other clients:

OpenSSL::X509::Certificate.new File.read ("openssl_test.crt")
TypeError: bad GENERALIZEDTIME format

this is from asn1parse:

  147:d=2  hl=2 l=  30 cons:   SEQUENCE          
  149:d=3  hl=2 l=  13 prim:    GENERALIZEDTIME   :201401010000Z
  164:d=3  hl=2 l=  13 prim:    UTCTIME           :390512144609Z

@hsbt hsbt mentioned this pull request Dec 7, 2016
Closed
@rhenium
Copy link
Member

rhenium commented Dec 7, 2016

Thanks for the PR!

Indeed, X.680 specifies that the seconds part can be absent. The asn1time_to_time() function also does not handle {UTC,Generalized}Time with fraction seconds or time zones, but this would be out of scope of this PR.

Could you add some basic tests to test/test_asn1.rb, and update the commit message to include the rationale about the change before merge?

@yogo1212
Copy link
Author

yogo1212 commented Dec 8, 2016

@rhenium ping :-)

@rhenium
Copy link
Member

rhenium commented Dec 9, 2016

@yogo1212 Nice, thanks! Could you squash the third commit into the first one?

@yogo1212
Copy link
Author

yogo1212 commented Dec 9, 2016

nothing comes more naturally than changing the course of history in the past :-D

@rhenium rhenium changed the base branch from master to maint December 10, 2016 06:47
@rhenium rhenium merged commit c083ff8 into ruby:maint Dec 10, 2016
@rhenium
Copy link
Member

rhenium commented Dec 10, 2016

Thanks!

@yogo1212 yogo1212 deleted the generalizedtime_format branch December 10, 2016 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yogo1212 @rhenium