[go: up one dir, main page]

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

DateTime::createFromFormat: Parsing TZID string is too greedy #9700

Closed
christoph-kluge opened this issue Oct 10, 2022 · 7 comments
Closed

DateTime::createFromFormat: Parsing TZID string is too greedy #9700

christoph-kluge opened this issue Oct 10, 2022 · 7 comments

Comments

@christoph-kluge
Copy link

Description

Discovered this issue while importing a data-set from a third party.

The following code:

<?php
var_dump(DateTime::createFromFormat('Y-m-d\TH:i:sP[e]', '2022-02-18T00:00:00+01:00[Europe/Berlin]'));

https://3v4l.org/W9SPY

Resulted in this output:

bool(false)

But I expected this output instead:

object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2022-02-18 00:00:00.000000"
  ["timezone_type"]=>
  int(1)
  ["timezone"]=>
  string(6) "+01:00"
}

PHP Version

8.1.9 (discovered with 7.2.24)

Operating System

No response

@iluuu1994
Copy link
Member

It seems the square brackets are the problem. 'Y-m-d\TH:i:sP e' works fine. https://3v4l.org/LJlNJ

@cmb69
Copy link
Member
cmb69 commented Oct 10, 2022

https://3v4l.org/4bcK8 clarifies; the timezone would be [Europe/Berlin].

@iluuu1994
Copy link
Member

Europe/Berlin] actually, probably. I'm assuming the parser just doesn't stop on ]. https://3v4l.org/PVnac

@cmb69
Copy link
Member
cmb69 commented Oct 10, 2022

Ah, right, @iluuu1994! Fixing exactly this issue would be trivial:

 ext/date/lib/parse_date.re | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/date/lib/parse_date.re b/ext/date/lib/parse_date.re
index 403f98d5ea..c2cdbc483b 100644
--- a/ext/date/lib/parse_date.re
+++ b/ext/date/lib/parse_date.re
@@ -754,7 +754,7 @@ static timelib_long timelib_lookup_abbr(const char **ptr, int *dst, char **tz_ab
 	timelib_long  value = 0;
 	const timelib_tz_lookup_table *tp;
 
-	while (**ptr != '\0' && **ptr != ')' && **ptr != ' ') {
+	while (**ptr != '\0' && **ptr != ')' && **ptr != ' ' && **ptr != ']') {
 		++*ptr;
 	}
 	end = *ptr;

Anyhow, this is a timelib issue, but I don't think we need to report upstream, since @derickr is the ext/date maintainer as well.

@hormus
Copy link
hormus commented Oct 13, 2022

It is a documentation problem DateTime::createFromFormat accepts the datetime string but is valid on the user side if the format is not repeated more than once, "p" and "e" or "e" and "e" is a repeat. Because the date to be created must be valid

<?php

var_dump(DateTime::createFromFormat('Y-m-d\TH:i:sP e e', '2022-02-18T00:00:00+23:00 Europe/Berlin Europe/Rome'));
var_dump(var_dump(DateTime::GetLastErrors()));

?>

Expected Result is Europe/Rome not Europe/Berlin or +23:00 (For me output date Expected is timezone: T from ISO 8601 , without Europe/Berlin or Europe/Rome ex1. 2022-02-17T23:00:00 or ex2. 2022-02-18T00:00:00+01:00)

This bug:

<?php

$date = DateTime::createFromFormat('Y-m-d H:i:s \[e\]', '2022-02-18 00:00:00 [Europe/Berlin]');
var_dump($date, is_object($date) ? $date->format('U') : NULL, is_object($date) ? $date->getOffset() : NULL);
var_dump(var_dump(DateTime::GetLastErrors()));

?>

It would be better to create a new format, instead of validating the final part of a string with the rest of the string
Special format: DateTimeImmutable::createFromFormat, date_create_immutable_from_format, DateTime::createFromFormat
and date_create_from_format: output date default hide, esplicit show: DateTimeInterface::format, DateTimeImmutable::format, DateTime::format
and date_format (function format) 'Y-m-d\TH:i:s output date[e output date]';

@derickr derickr changed the title DateTime::createFromFormat with two identical timezones resolves to false DateTime::createFromFormat: Parsing TZID string is too greedy Oct 18, 2022
@derickr derickr self-assigned this Oct 18, 2022
@derickr
Copy link
Member
derickr commented Oct 18, 2022

This is fixed in timelib now — I'll release and then merge this into PHP before the release of 8.0.26 and 8.1.13

derickr/timelib@cca0fe4

derickr added a commit to derickr/php-src that referenced this issue Nov 30, 2022
@derickr
Copy link
Member
derickr commented Nov 30, 2022

Now fixed, for PHP 8.1.14 and PHP 8.2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@derickr @christoph-kluge @iluuu1994 @cmb69 @hormus and others