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

Difficulties in parsing HTTP Dates with 0.2.1 #200

Closed
kevinpoitra opened this issue Jan 8, 2020 · 5 comments
Closed

Difficulties in parsing HTTP Dates with 0.2.1 #200

kevinpoitra opened this issue Jan 8, 2020 · 5 comments
Assignees
Labels
C-bug Category: bug in current code

Comments

@kevinpoitra
Copy link

I'm working on helping actix-web upgrade to 0.2.1. Part of that upgrade entails that it still parses the three different HTTP Date time formats correctly, those being:

  • RFC 1123
  • RFC 850
  • ANSI C's asctime

In time 0.1, there were parsed in the following ways, in order:

  • time::strptime("Sun, 07 Nov 1994 08:48:37 GMT", "%a, %d %b %Y %T %Z")
  • time::strptime("Sunday, 07-Nov-94 08:48:37 GMT", "%A, %d-%b-%y %T %Z")
  • time::strptime("Sun Nov 7 08:48:37 1994", "%c")

Being that the format patterns changed in 0.2, I've been working on converting those old patterns into new patterns.

For RFC 1123 and RFC 850, I make the assumption based on text from the RFC that any received HTTP Date will always be in GMT, and drop the now non-existent %Z pattern. Additionally, %T changed as well (from %H:%M:%S to %-H:%M:%S), so I changed it to just plainly use %H:%M:%S, making both of these patterns %a, %d %b %Y %H:%M:%S and %A, %d-%b-%y %H:%M:%S, respectively. This works well - both of the time strings above can be successfully parsed. For RFC 850's two digit years, I also employ the logic outlined in Section 19.3 of RFC 2616 to make it into a full length year.

The last one with %c, however, is seemingly not possible to recreate in 0.2. In 0.2, %c changed to %a %b %-d %-H:%M:%S %-Y. This doesn't work, as %-d means that the day value is not padded, which is not compliant with asctime. To clarify:

In time 0.1, time::strptime("Sun Nov 7 08:48:37 1994", "%c") would output a Tm struct that looked like so:

Tm {
	tm_sec: 37,
	tm_min: 48,
	tm_hour: 8,
	tm_mday: 7,
	tm_mon: 10,
	tm_year: 94,
	tm_wday: 0,
	tm_yday: 0,
	tm_isdst: 0,
	tm_utcoff: 0,
	tm_nsec: 0
}

The following 0.2 code, however, produces an Err(InvalidDayOfMonth) error:

PrimitiveDateTime::parse("Sun Nov  7 08:48:37 1994", "%c")

In an attempt to recreate 0.1's %c functionality in 0.2, I came up with this pattern: %a %b %_d %H:%M:%S %Y. This, to me, should work - the underscore within %d should pad the day with a space if the day value is only one digit like in the above example, however, it produces an Err(UnexpectedCharacter { expected: ' ', actual: '0' }) instead. When using a two-digit day (e.g: Sun Nov 17 08:48:37 1994), it parses successfully. Being that it only fails when the day is space padded, this seems like a bug to me.

For what it's worth, it also looks like it's only the underscore modifier that's having an issue - using %-d with a non-padded day works just fine (7 and 17 parse successfully), and despite %d already being zero padded, %0d works for zero padded days (07 and 17).

@jhpratt
Copy link
Member

jhpratt commented Jan 8, 2020

I'll take a look into this. Just from quickly reading this, it sounds like this is a bug in parsing.

@jhpratt jhpratt self-assigned this Jan 8, 2020
@jhpratt jhpratt added the C-bug Category: bug in current code label Jan 8, 2020
@jhpratt
Copy link
Member

jhpratt commented Jan 8, 2020

The UnexpectedChar actually comes from after the 7. The code currently consumes the padding, but then still tries to consume two digits (instead of just one). This is a one line fix that I'll push up and release as v0.2.2 in a few minutes.

@jhpratt jhpratt closed this as completed in 208a3d3 Jan 8, 2020
@jhpratt
Copy link
Member

jhpratt commented Jan 8, 2020

Published!

@kevinpoitra
Copy link
Author

Thank you! %a %b %_d %H:%M:%S %Y seems to be working as expected now.

@jhpratt
Copy link
Member

jhpratt commented Jan 8, 2020

I checked your case specifically! Let me know if there are any other issues you run into; I'm more than willing to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug in current code
Projects
None yet
Development

No branches or pull requests

2 participants