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

GetTimestamp might delete a field that isn't a timestamp #239

Closed
kentquirk opened this issue Jan 19, 2022 · 0 comments · Fixed by #263
Closed

GetTimestamp might delete a field that isn't a timestamp #239

kentquirk opened this issue Jan 19, 2022 · 0 comments · Fixed by #263

Comments

@kentquirk
Copy link
Contributor

In the loop at the bottom of GetTimestamp(), the function records a name to delete, and then attempts to parse its value. The result of that parse will be a zero Time value if the parse failed (and if so, the loop continues). But if the loop runs without ever finding a valid timestamp, it will still delete the last field that matched one of the names it's searching.

To be fair, if someone has called a field "Timestamp" and this code can't parse it, there are probably bigger problems. But I noticed this while reviewing the code.

kentquirk added a commit that referenced this issue Aug 26, 2022
## Which problem is this PR solving?

- Fixes #238
- Fixes #239 

## Short description of the changes

- GetTimestamp violates its documented behavior (fails to delete the timestamp field) if a timestamp matches a certain format. This causes it to do that.
- GetTimestamp looks at a bunch of fields and tries to parse them as a timestamp. If it succeeds, it deletes the field. But there was a bug (see the referenced bug report). This fixes that bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant