From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A27823858C51 for ; Mon, 16 May 2022 17:38:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A27823858C51 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652722704; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=P7uM3hs0AMTf/WXhMEkxctzNhpvtblWvDshUatR+NRQ=; b=XjATnCg6ujHAEwnh6SP0tUaaFSUyYdM4iaF+UO4x8k7WLlQHRUycsUsNJcE1ew4MHVLy5T bQgdZMBFg0dPMtJlKhke8Wu1nLhSm9ihXZgp6yyd3XiuLuhwxB921YU0JmEISE959g6leI prTIDTJrg5De+LNI345Sc0m3/s/hGf0= Received: from mail-yw1-f198.google.com (mail-yw1-f198.google.com [209.85.128.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-519-DHu6fDrjOsydnYdKNdXsdg-1; Mon, 16 May 2022 13:38:22 -0400 X-MC-Unique: DHu6fDrjOsydnYdKNdXsdg-1 Received: by mail-yw1-f198.google.com with SMTP id 00721157ae682-2fef32314f7so29914007b3.18 for ; Mon, 16 May 2022 10:38:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=P7uM3hs0AMTf/WXhMEkxctzNhpvtblWvDshUatR+NRQ=; b=U/I92n7yd5R8RtJLwrTGIFA0bdLKxOsX9zW/WKT/xEGE6YCbuHnnw0AlnrDUl07McI Fit08JchjWFji+YWyDr6WwIKJCVPLMGCJn/lZLqZDPUo834/T8Tk6nlORwcte2IGjVgL PFRkSMJi/2/0RnipsaKpkHWhGpz44s57AQegfBX2KLn3OIRO+E8KPvKb57LEBuxiQUiS 4jXAGpqy9+cevbACmG2Nng/uZcn7563tgQI/1pa71DveJQQKwJI+q7KERnYmgLaGzgU6 bkNhR8qYsNDxrZUGo6K+7vrSD15mIVh20Pc2Nl0nn6PBF5Oi74tarFGvVlO6Y3t9lrik reCw== X-Gm-Message-State: AOAM532RZGcjT6q9Rrignamn3+w8DOV6LdbG2WRjVwlNRbZzdvHFrlxQ 9YVtiyMbmQNB1FW5c2KUkVHgp6JCLX2FuitZ0jad5yH7Mib2J8sE03nzM0hoHzrJP1+DmYcqTV5 xYbOEanj+8NPKKnfDxNlkC4Szz8t3/m8= X-Received: by 2002:a81:2008:0:b0:2f8:3968:e70a with SMTP id g8-20020a812008000000b002f83968e70amr21722369ywg.321.1652722702090; Mon, 16 May 2022 10:38:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzxa1Hmisu2RkmRAlsEQGZQgMY7llMppt7qzepaJ+aX0PE8WdSzspsQloIWy5HX4tbEQSC1Mn5wA36bn9oFRYM= X-Received: by 2002:a81:2008:0:b0:2f8:3968:e70a with SMTP id g8-20020a812008000000b002f83968e70amr21722346ywg.321.1652722701859; Mon, 16 May 2022 10:38:21 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: From: Jeff Johnston Date: Mon, 16 May 2022 13:38:10 -0400 Message-ID: Subject: Re: [PATCH] add tests for tzset(3) To: Dimitar Dimitrov Cc: jdoubleu , Newlib Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jjohnstn@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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 17:38:27 -0000 On Mon, May 16, 2022 at 12:06 PM Dimitar Dimitrov wrote: > 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, righ= t? > Yes. > > > 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 th= em > > 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 > > --- > > =F0=9F=99=8E=F0=9F=8F=BB=E2=80=8D=E2=99=82=EF=B8=8F 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 n= ot > > > 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, righ= t? > > > > > > > > > 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.c= sv > > > [6]: > https://sourceware.org/git/?p=3Dnewlib-cygwin.git;a=3Dblob;f=3Dnewlib/lib= c/time/tzset_r.c;h=3D9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=3DHEAD#l13 > > > > > > [7]: > https://sourceware.org/git/?p=3Dnewlib-cygwin.git;a=3Dblob;f=3Dnewlib/lib= c/time/tzset_r.c;h=3D9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=3DHEAD#l57 > > > > > > > > > > > > > > > Cheers > > > --- > > > =F0=9F=99=8E=F0=9F=8F=BB=E2=80=8D=E2=99=82=EF=B8=8F 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 i= s > a > > > > > parse failure. > > > > =E2=9C=94 Check =E2=9C=94 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 agains= t > > > > the newlib philosophy to keep things limited and under control to > > > > support small targets. Larger targets like Cygwin (do our own thin= g > > > > 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 t= o > > > > 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 d= o > > > > 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. > >