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

fix: escape last period to match only milliseconds (#1239) #1295

Merged
merged 3 commits into from
Jan 3, 2021

Conversation

bcurtin144
Copy link
Contributor

@bcurtin144 bcurtin144 commented Dec 31, 2020

fix #1239
To be sure the last digit token in REGEX_PARSE only matches for milliseconds, the period must be escaped. Otherwise, it acts as a wildcard.

All of these are the same date/time using different ISO 8601 time zone formats:
dayjs("2020-12-31T18:00:00.000-0500")
dayjs("2020-12-31T18:00:00-05:00")
dayjs("2020-12-31T18:00:00-0500")
The first two parse correctly because REGEX_PARSE doesn't match. But the last example does match REGEX_PARSE, with the final - character matching the wildcard. So the date is parsed in local time and the first three characters of 0500 become 50ms.

@codecov
Copy link

codecov bot commented Jan 1, 2021

Codecov Report

Merging #1295 (a02327a) into dev (de49bb1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1295   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          174       174           
  Lines         1672      1672           
  Branches       382       382           
=========================================
  Hits          1672      1672           
Impacted Files Coverage Δ
src/constant.js 100.00% <100.00%> (ø)

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 de49bb1...a02327a. Read the comment docs.

@iamkun
Copy link
Owner

iamkun commented Jan 1, 2021

Can you add some test case please ? like https://github.com/iamkun/dayjs/blob/dev/test/parse.test.js#L138

includes two tests that would have matched before fix iamkun#1239 but no longer match
@bcurtin144
Copy link
Contributor Author

Added some test cases. The regular expression can still get in trouble accepting odd strings like 2021-012T123:00 but that's probably for another day.

@iamkun
Copy link
Owner

iamkun commented Jan 3, 2021

LGTM

@iamkun
Copy link
Owner

iamkun commented Jan 3, 2021

we should only match the last . as a simple character
e.g. 2019-03-25T06:41:00[.]999999999

But not as a wildcard, this would cause some bug in
2020-12-31T18:00:00[-]0500

@iamkun iamkun merged commit 90bb665 into iamkun:dev Jan 3, 2021
iamkun pushed a commit that referenced this pull request Jan 3, 2021
# [1.10.0](v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](#1266)) ([fd229fa](fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](#1255)) ([f2e4790](f2e4790))
* add type support for plural forms of units ([#1289](#1289)) ([de49bb1](de49bb1))
* escape last period to match only milliseconds ([#1239](#1239)) ([#1295](#1295)) ([64037e6](64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](#1298)) ([f63375d](f63375d)), closes [#598](#598) [#313](#313)
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
# [1.10.0](iamkun/dayjs@v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](iamkun/dayjs#1266)) ([fd229fa](iamkun/dayjs@fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](iamkun/dayjs#1255)) ([f2e4790](iamkun/dayjs@f2e4790))
* add type support for plural forms of units ([#1289](iamkun/dayjs#1289)) ([de49bb1](iamkun/dayjs@de49bb1))
* escape last period to match only milliseconds ([#1239](iamkun/dayjs#1239)) ([#1295](iamkun/dayjs#1295)) ([64037e6](iamkun/dayjs@64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](iamkun/dayjs#1298)) ([f63375d](iamkun/dayjs@f63375d)), closes [#598](iamkun/dayjs#598) [#313](iamkun/dayjs#313)
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
# [1.10.0](iamkun/dayjs@v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](iamkun/dayjs#1266)) ([fd229fa](iamkun/dayjs@fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](iamkun/dayjs#1255)) ([f2e4790](iamkun/dayjs@f2e4790))
* add type support for plural forms of units ([#1289](iamkun/dayjs#1289)) ([de49bb1](iamkun/dayjs@de49bb1))
* escape last period to match only milliseconds ([#1239](iamkun/dayjs#1239)) ([#1295](iamkun/dayjs#1295)) ([64037e6](iamkun/dayjs@64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](iamkun/dayjs#1298)) ([f63375d](iamkun/dayjs@f63375d)), closes [#598](iamkun/dayjs#598) [#313](iamkun/dayjs#313)
splashwizard pushed a commit to splashwizard/tracking-time that referenced this pull request Oct 21, 2024
# [1.10.0](iamkun/dayjs@v1.9.8...v1.10.0) (2021-01-03)

### Bug Fixes

* add ordinal to localeData plugin ([#1266](iamkun/dayjs#1266)) ([fd229fa](iamkun/dayjs@fd229fa))
* add preParsePostFormat plugin & update Arabic [ar] locale ([#1255](iamkun/dayjs#1255)) ([f2e4790](iamkun/dayjs@f2e4790))
* add type support for plural forms of units ([#1289](iamkun/dayjs#1289)) ([de49bb1](iamkun/dayjs@de49bb1))
* escape last period to match only milliseconds ([#1239](iamkun/dayjs#1239)) ([#1295](iamkun/dayjs#1295)) ([64037e6](iamkun/dayjs@64037e6))

### Features

* add ES6 Module Support, package.json module point to "esm/index.js" ([#1298](iamkun/dayjs#1298)) ([f63375d](iamkun/dayjs@f63375d)), closes [#598](iamkun/dayjs#598) [#313](iamkun/dayjs#313)
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.

dayjs.utc() dropped timezone offset support
2 participants