From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server28.superhosting.bg (server28.superhosting.bg [217.174.156.11]) by sourceware.org (Postfix) with ESMTPS id 096AD3858C55 for ; Mon, 16 May 2022 16:05:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 096AD3858C55 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dinux.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dinux.eu DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=dinux.eu; s=default; h=In-Reply-To:Content-Transfer-Encoding:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=A1fjEgLhHvAglK+JDNxu5VA0afs5+jAmcN1+jiOZn0U=; b=gd9I+j4LQGVbRqTgSm/3kzY1E7 iWA7uSk8GVm/ulRdfAkwB4pFMtRlADdKtr1JytAM++/YWyVAXq/75zMbH5I8SUoSJL/7kj0nPpYhY /GC3RqjuRk1e/lzjLXTWLGhL51FEnWnXH+c8PFPgyH+Aoh3LCm/cNG5ZHc9XA/alY5FXem/0kg2Tt u3F1nIM82HqrQXSbRRCvVg5LUuT4jXACARbkjLuoT9wJSQar7JAzQZ1WWjhvx1k+swq9/MF0A3RDF jO036brtu1NPd68MhakjgddSlRjVeY/zSPrd4jGnL1724MPQbXv3GIltKQIO0bFYHNSb4p/AF3/Om /oAV2pmg==; Received: from [95.87.234.74] (port=36492 helo=localhost) by server28.superhosting.bg with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nqdDz-0000b1-EN; Mon, 16 May 2022 19:05:38 +0300 Date: Mon, 16 May 2022 19:05:34 +0300 From: Dimitar Dimitrov To: jdoubleu Cc: newlib@sourceware.org, Jeff Johnston Subject: Re: [PATCH] add tests for tzset(3) Message-ID: References: <822e81a0-ed9f-200e-3318-0495456ad67e@SystematicSw.ab.ca> <20220407233425.2012-1-Brian.Inglis@SystematicSW.ab.ca> <9603e3aa-bd7d-6740-c710-27ace1d80397@SystematicSw.ab.ca> <25cfc7a2-2c66-f9fe-581b-8d3cec5d3bd9@jdoubleu.de> <426d1e93-6c4f-4ccc-9522-c187e10716f0@jdoubleu.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <426d1e93-6c4f-4ccc-9522-c187e10716f0@jdoubleu.de> X-OutGoing-Spam-Status: No, score=-2.9 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server28.superhosting.bg X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - dinux.eu X-Get-Message-Sender-Via: server28.superhosting.bg: authenticated_id: dimitar@dinux.eu X-Authenticated-Sender: server28.superhosting.bg: dimitar@dinux.eu X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2022 16:05:45 -0000 On Sat, May 14, 2022 at 04:39:16PM +0200, jdoubleu wrote: > > To summarize, the following cases are errors: > > 1. name is too short (less than 3 chars) > > 2. name is too long (more than TZNAME_MAX) > > 3. name includes arbitrary chars (not <>+-ALPHANUM) > > In all of these error cases, the time should be set back to UTC, right? > > Here's my patch which adds test cases for the above error cases. Did you forget to attach the actual patch with updated test cases? I can't find it. Regards, Dimitar > > It's based on the latest (pending) changes of tzset[1]. Please apply them > first. > > I wasn't able to run the tests, yet. With glibc they're obviously failing. > > Could you please test them? > > > > [1]: https://sourceware.org/pipermail/newlib/2022/019581.html > > > Cheers > --- > ๐Ÿ™Ž๐Ÿปโ€โ™‚๏ธ jdoubleu > On 4/14/2022 10:59 AM, jdoubleu wrote: > > On 2022-04-13 14:33, Jeff Johnston wrote: > > > Looking at the glibc tzset code I have locally (not latest/greatest, but > > > does support angle brackets): > > > > I can confirm the behavior with glibc[1]. As it turns out, glibc does > > not directly impose a character limit on the timezone name, but requires > > at least 3 characters. From the man page[2]: > > > > > The std string specifies an abbreviation for the timezone and must be > > > three or more alphabetic characters. > > > > To my misunderstanding, they don't even ignore remaining characters, but > > keep all of them, as you can see in the output[1] and Jeff Johnston > > explained. > > > > > but you imply that glibc in fact uses the equivalent of the scanf > > > "%m[...]" (malloc) > modifier, and I think using that would be > > > against the newlib > > philosophy to keep things > > > limited and under control to support small targets. > > > > I agree, newlib SHOULD impose a limit. Especially, since the POSIX > > standard[3] already introduces an upper limit, though unspecified. > > > > The current limit is 11 characters, if I'm not mistaken. The longest > > name from the tzdb[4] is "<+1030>" i.e. 5 chars (see all extracted > > names[5]). All others usually are 3 or 4 chars long. > > > > That said, I think 11 is reasonably large enough. > > > > However, it could be helpful to get the limit from user-code, because > > there is no error reporting mechanism used. Right now, the limit is only > > defined in tzset_r.c[6]. So maybe move it to limits.h? One thing to not > > forget here is to keep limit in sync with the sscanf format's maximum > > field width[7]. > > > > > > To summarize, the following cases are errors: > > 1. name is too short (less than 3 chars) > > 2. name is too long (more than TZNAME_MAX) > > 3. name includes arbitrary chars (not <>+-ALPHANUM) > > In all of these error cases, the time should be set back to UTC, right? > > > > > > I'm going to prepare some test cases for the test suite to check for the > > errors as well. > > > > > > [1]: https://godbolt.org/z/o93zo3qxv > > [2]: https://www.man7.org/linux/man-pages/man3/tzset.3.html > > [3]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03 > > > > [4]: https://github.com/eggert/tz > > [5]: > > https://raw.githubusercontent.com/nayarsystems/posix_tz_db/master/zones.csv > > [6]: https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/tzset_r.c;h=9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=HEAD#l13 > > > > [7]: https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/tzset_r.c;h=9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=HEAD#l57 > > > > > > > > > > Cheers > > --- > > ๐Ÿ™Ž๐Ÿปโ€โ™‚๏ธ jdoubleu > > On 4/14/2022 12:19 AM, Brian Inglis wrote: > > > On 2022-04-13 14:33, Jeff Johnston wrote: > > > > Looking at the glibc tzset code I have locally (not latest/greatest, but > > > > does support angle brackets): > > > > > > > > If there any parse failures, UTC is defaulted. > > > > > > We currently leave the time zone info unchanged. > > > > > > > Extraneous characters inside brackets or less than 3 characters is a > > > > parse failure. > > > โœ” Checkย ย ย  โœ” Check > > > > > > > Glibc parses the tz name string char by char and allocates space for > > > > the name strings so there is no max size. > > > > > > The suggestion was that glibc ignores the remaining characters, but > > > you imply that glibc in fact uses the equivalent of the scanf > > > "%m[...]" (malloc) modifier, and I think using that would be against > > > the newlib philosophy to keep things limited and under control to > > > support small targets.ย  Larger targets like Cygwin (do our own thing > > > including zoneinfo), and perhaps RTEMS, can supply their own > > > enhancements. > > > > > > > the name strings so there is no max size.ย  I am fine if you want > > > > to mandate a maximum, but if you do, then too many chars should > > > > be treated as a failure.ย  If you aren't certain of the limit, > > > > make the limit higher than you expect. > > > Current limits are 3-10 allowing for e.g. which is the > > > most ever likely to be used. It might be reasonable to bump it up to > > > say 15. > > > > > > > If people run into max limit with reasonable timezone format > > > > strings, then > > > > we can up the limit. > > > > > > The conditions are more or less what is implemented, but we could do > > > with a couple more tweaks to improve things, like check for more or > > > extraneous chars within the bracket quotes, and that no characters > > > remain unconsumed at the end of the parse.