From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 5B874AB1E for ; Wed, 28 Jun 2023 09:27:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4325F1EBAB for ; Wed, 28 Jun 2023 09:27:37 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 28 Jun 2023 09:27:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EF48941A50; Wed, 28 Jun 2023 09:27:35 +0200 (CEST) Message-ID: <2c99e898-7a3c-2f7e-61df-386dd321a205@proxmox.com> Date: Wed, 28 Jun 2023 09:27:35 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Stoiko Ivanov , pmg-devel@lists.proxmox.com References: <20230627194650.34027-1-s.ivanov@proxmox.com> <20230627194650.34027-2-s.ivanov@proxmox.com> From: Dominik Csapak In-Reply-To: <20230627194650.34027-2-s.ivanov@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [main.rs] Subject: Re: [pmg-devel] [RFC pmg-log-tracker 1/2] rfc3339 timestamps: counter-offset timezone X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jun 2023 07:27:37 -0000 High level comment: isn't the api simply adding the local offset from 'now' ? so here we add the offset depending on when it was logged, meaning if the offset changed from the log to now it's still off to what it was previously? e.g. log at 05:00 with +02:00 (epoch at 03:00 UTC) gets converted to epoch at 05:00 UTC now it's +01:00 (e.g. after summer/wintertime change) result from the api is epoch at 04:00 UTC so i guess we should either add the current timezone offset (since that's what the api does, or fix it 'correct' by adding the timezone info directly in the log-tracker for lines where we don't have the info, and drop it in the api... otherwise the code looks ok with one nit inline On 6/27/23 21:46, Stoiko Ivanov wrote: > the old time-format had no timezone-information so the API treats the > result as 'timezoned-epoch' and subtracts the timezone-offset > > In order to support both formats add the timezone-offset to the > already offset time - so that the subtraction in the API is canceled > out. > > This can luckily be dropped once we drop support for traditional > syslog format. > > inspired by parse_rfc3339_do in proxmox-time > > Signed-off-by: Stoiko Ivanov > --- > src/main.rs | 27 +++++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/src/main.rs b/src/main.rs > index e7bffd8..6c4a555 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -2263,12 +2263,31 @@ fn parse_time_with_year(data: &'_ [u8]) -> Option<(time_t, &'_ [u8])> { > timestamp_buffer[0..year_time_len].copy_from_slice(year_time); > timestamp_buffer[year_time_len..timestamp_len].copy_from_slice(timezone); > > - match proxmox_time::parse_rfc3339(unsafe { > + let epoch = match proxmox_time::parse_rfc3339(unsafe { > std::str::from_utf8_unchecked(×tamp_buffer[0..timestamp_len]) > }) { > - Ok(ltime) => Some((ltime, data)), > - Err(_err) => None, > - } > + Ok(ltime) => ltime, > + Err(_err) => return None, > + }; > + > + // FIXME: the traditional syslog format does not have timezone information, so the api-backend > + // shifts the time by the offset (i.e. parse_time_no_year returns timestamps in local time) > + // readd the TZ-offset here to match the broken format > + // drop after legacy parsing is dropped. > + let tz = timezone[0]; > + if tz == b'Z' { > + return Some((epoch, data)); > + } > + > + let hours = (timezone[1] as i32 - 48) * 10 + (timezone[2] as i32 - 48); > + let mins = (timezone[4] as i32 - 48) * 10 + (timezone[5] as i32 - 48); nit: i guess it's copied but wouldn't it make more sense to use b'0' instead of 48 here? at least it would it make more obvious what it does > + let offset = hours * 3600 + mins * 60; > + let epoch = match tz { > + b'+' => epoch + offset as i64, > + b'-' => epoch - offset as i64, > + _ => unreachable!(), // already checked in parse_rfc3339 > + }; > + Some((epoch, data)) > } > > fn parse_time_no_year(data: &'_ [u8], cur_year: i64, cur_month: i64) -> Option<(time_t, &'_ [u8])> {