On 28/05/2024 15:05, Patrick Steinhardt wrote:

On Mon, May 27, 2024 at 09:17:06AM +0000, darcy via GitGitGadget wrote:

quoted

From: darcy <redacted>

diff --git a/date.c b/date.c
index 7365a4ad24f..8388629f267 100644
--- a/date.c
+++ b/date.c

@@ -908,7 +908,7 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
  			match = match_alpha(date, &tm, offset);
  		else if (isdigit(c))
  			match = match_digit(date, &tm, offset, &tm_gmt);
-		else if ((c == '-' || c == '+') && isdigit(date[1]))
+		else if ((c == '-' || c == '+') && isdigit(date[1]) && tm.tm_hour != -1)
  			match = match_tz(date, offset);

Without having a deep understanding of the code I don't quite see the
connection between this change and the problem description. Is it
necessary? If so, it might help to explain why it's needed in the commit
message or in the code.

I was wondering about this change too

quoted

  		if (!match) {

@@ -937,8 +937,13 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
  		}
  	}
  
-	if (!tm_gmt)
+	if (!tm_gmt) {
+		if (*offset > 0 && *offset * 60 > *timestamp) {
+			return -1;
+		}

Nit: we don't add curly braces around one-line conditional bodies.

This change here is the meat of it and looks like I'd expect.

quoted

  		*timestamp -= *offset * 60;

Do we also want to check for overflows in the other direction (a large 
timestamp with a negative timezone offset)?

Best Wishes

Phillip