[PUP-7757] Successful parsing of timestamp should raise error unless complete string is parsed Created: 2017/07/06  Updated: 2017/08/18  Resolved: 2017/07/10

Status: Closed
Project: Puppet
Component/s: None
Affects Version/s: PUP 4.10.4, PUP 5.0.0
Fix Version/s: PUP 5.1.0

Type: Improvement Priority: Normal
Reporter: Thomas Hallgren Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relates to PUP-7753 Timestamp doesn't provide a default f... Closed
Sub-team: Language
Team: Agent
Story Points: 1
Sprint: Agent 2017-07-12
Release Notes: Bug Fix
Release Notes Summary: If a {{Timestamp}} was created from a string with trailing unrecognized characters there would be no error and the produced {{Timestamp}} would be based on only the recognized part. This will now instead raise an error.
QA Risk Assessment: No Action
QA Risk Assessment Reason: Covered by spec tests


The following puppet expression is valid:

notice(Timestamp('1969-08-20T20:18:00 UTC, one giant leap for mankind'))

will run without errors and notice:

1969-08-20T20:18:00.000000000 UTC

This is because one of the default formats successfully parsed the string and managed to satisfy all of its format specifiers.

It would be desirable if Puppet could detect that the string contains trailing garbage and raise an error. Doing so is not trivial however. The Timestamp class currently relies on the DateTime#strptime function which provides no means to detect the garbage so solving this means writing a new parser.

Comment by Henrik Lindberg [ 2017/07/06 ]

There is an _strptime that returns a hash with information - does it by any chance include more information?

Comment by Henrik Lindberg [ 2017/07/06 ]

Meh, just looked in the source, it is already using that method.

Comment by Henrik Lindberg [ 2017/07/06 ]

Actually - it is not using _strptime everywhere. If it did you would find the problem.

DateTime._strptime(str, fmt)
{:year=>1969, :mon=>8, :mday=>20, :hour=>20, :min=>18, 
 :sec=>0, :zone=>"UTC", :offset=>0,
 :leftover=>", one giant leap for mankind"}

Comment by Henrik Lindberg [ 2017/07/06 ]

Since _strptime is undocumented we need to add a unit test that asserts that it does not change in a surprising way.

Comment by Thomas Hallgren [ 2017/07/06 ]

Good catch. I actually looked at that hash, as described in strptime(3) (which is what the docs refer to). It doesn't contain the leftover field. So it's Ruby specific, and it's undocumented. Sigh...

Comment by Josh Cooper [ 2017/07/08 ]

Merged to puppet#master in https://github.com/puppetlabs/puppet/commit/5337f09124f3955e34b40a5e5f7e16e1a7ba6202

Generated at Tue Aug 11 00:30:34 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.