Revert "Unescaped left brace in regex" fatality #15792
Comments
From @andkI apologize that I did not wake up when I was reading the following
> On 12/08/2016 01:08 PM, Sawyer X wrote:
> Yes I would like to call up and suggest that now is the last moment before There is a portion of the CPAN that remained untested throughout the -- perl -V Summary of my perl5 (revision 5 version 25 subversion 9) configuration: Characteristics of this binary (from libperl): |
From @jkeenanOn Tue, 03 Jan 2017 22:00:53 GMT, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
Can you identify those distributions? Thank you very much. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @andk
> Can you identify those distributions? | distro | dependents | comment | The list has been compiled manually and may have wrong positives, You get the dependents via http://deps.cpantesters.org/depended-on-by.pl Thanks, |
From @jkeenanOn Wed, 04 Jan 2017 07:49:51 GMT, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
Andreas, thanks for providing that list. I'm attaching a reduced version of that list holding only those distributions where the dependencies count > 0. Using cpanm, I was able to install both CHI and PerlX-Assert at blead. That leaves 19 unpatched distributions. I suspect you've probably filed bug reports for each of these (correct me if that's wrong). Indeed, I know that I've submitted patches for some of these. So the questions we face are: * How do we get the maintainers of these 19 distros to fix the fatal-brace errors so that we can then go on to perform regular CPANtesters runs on each of their respective dependencies? * What are the advantages/disadvantages to the Perl ecosystem if we revert, i.e., if we do not proceed to fatalize the unescaped left brace in perl-5.26? * Conversely, what are the advantages/disadvantages to the Perl ecosystem if we do not revert, i.e., we proceed as planned to fatalize in perl-5.26? Thank you very much. |
From @jkeenanOn Wed, 04 Jan 2017 13:29:49 GMT, jkeenan wrote:
Re-attaching file with .txt extension to make it more visible. -- |
From @jkeenandistro|dependents|comment| |
From [Unknown Contact. See original ticket]On Wed, 04 Jan 2017 13:29:49 GMT, jkeenan wrote:
Re-attaching file with .txt extension to make it more visible. -- |
From @jkeenanOn Wed, 04 Jan 2017 13:29:49 GMT, jkeenan wrote:
I can confirm that you, Slaven or I have submitted bug reports or created github issues for each of the 19 distributions with dependencies. So we need to determine who to get those issues addressed and new CPAN releases issued in a timely manner. Thank you very much. |
From cpan@zoffix.comNoticed this ticket by accident... Shipped fixed version of ZOFFIX/App-ZofCMS to CPAN |
From [Unknown Contact. See original ticket]Noticed this ticket by accident... Shipped fixed version of ZOFFIX/App-ZofCMS to CPAN |
From @khwilliamsonOn 1/3/2017 3:42 PM, James E Keenan via RT wrote:
I believe he means, which distributions are the ones that are preventing
|
From @jkeenanOn Thu, 05 Jan 2017 14:07:29 GMT, khw@indra.com wrote:
Well, what I was actually hoping for was a list of the distributions which *have been prevented from being tested*. My premise has been that it was the distros in Andreas's list with > 0 dependencies which are doing the preventing. Was that premise incorrect?
-- |
From @jkeenanOn Thu, 05 Jan 2017 14:07:29 GMT, khw@indra.com wrote:
If we go that route -- and to a certain extent I think we will have to -- here is some suggested language (attachment).
-- |
From @jkeenanThis is a bug report for: __________ Today I attempted to build and test this distribution against Perl 5 I would recommend that you correct these problems at your earliest 1. So that your distribution passes all tests when perl-5.26.0 is 2. Other CPAN distributions depend on yours. In preparation for the http://deps.cpantesters.org/depended-on-by.pl?dist=__________ By correcting this problem, you will be helping to ensure that both your Thank you very much. |
From @jkeenanOn Wed, 04 Jan 2017 13:34:03 GMT, jkeenan wrote:
App-ZofCMS maintainer responded to pull request. This distro now PASS. 1 down; 18 (or 20, depending on how you count) to go. -- |
From @xsawyerxOn 01/03/2017 11:50 PM, Karl Williamson wrote:
We need to decide on an appropriate deadline for reverting, in case the |
From @xsawyerxOn 01/05/2017 04:15 PM, James E Keenan via RT wrote:
A more succinct version. What do you think? (Attached.) |
From @xsawyerx# This is a bug report for: Due to unescaped left braces in some regular expression patterns in We would appreciate help in sorting this out before the new version of I'm including a log file of my failure to build and install using Here is a link to all modules that depend on yours: Thanks! |
From @jkeenanOn Fri, 06 Jan 2017 09:36:01 GMT, xsawyerx@gmail.com wrote:
##### As I look at this (and similar language in my original version), I'm thinking this is too lenient. A maintainer reading it might think he/she has until mid-May to fix the braces and do a new CPAN release. Because this blocks the testing of other CPAN distros, I think we should suggest a deadline: 2 weeks after this message is added to the existing bug ticket. Thank you very much. -- |
From @jkeenanOn Fri, 06 Jan 2017 09:19:35 GMT, xsawyerx@gmail.com wrote:
My recommendations stem from a desire to *not* revert the fatal-left-brace change and a willingness to consequently sustain some breakage when 5.26.0 is released. YMMV. 1. Two distros were special-cased in Andreas's list: CHI and PerlX-Assert. Get clarification from Andreas and Slaven as to what the CPANtester-related problems and workarounds are. 2. For the other 18 distros with dependencies, post in each existing rt.cpan.org ticket or github.com pull request a notice with a request for action within two weeks. 3. For the distros which have the problem but which, in Andreas's list, have 0 dependencies, post a gentler message in the ticket/pull request reminding the maintainer of the problem. (But we won't take further action.) 4. After two weeks, repeat post in (2) as needed. Consider possibility of notifying the maintainers of the dependent distros that CPAN testing of their distros is blocked. 5. Of the 18 remaining distros with dependencies, 11 have N > 1 dependencies. If we get a majority -- 6 -- of those distros corrected and re-released, then, IMO, we should be content with our efforts and clear this as a blocker to 5.26.0. Thank you very much. -- |
From @xsawyerxThis failed yesterday, so I'm trying to resend it again today. On 01/06/2017 02:42 PM, James E Keenan via RT wrote:
This sounds like a solid plan to me. Thank you for spearheading this. |
From @xsawyerxThis failed yesterday, so I'm trying it again today. On 01/06/2017 02:27 PM, James E Keenan via RT wrote:
The reason I picked a shorter version (and perhaps shorter still would I wouldn't be surprised if some would not be able to make the change |
From @LeontOn Fri, Jan 6, 2017 at 2:42 PM, James E Keenan via RT <
Quite frankly I think this is not the sort of change that should be Leon |
From @xsawyerxOn 01/09/2017 11:51 PM, Leon Timmermans wrote:
If not 5.26, do you see it then being fatal in 2.28, 5.30, or when? |
From @LeontOn Tue, Jan 10, 2017 at 12:26 PM, Sawyer X <xsawyerx@gmail.com> wrote:
According to the document you posted a few days ago, it's on schedule for Leon |
From @jkeenanOn Tue, 10 Jan 2017 18:13:38 GMT, LeonT wrote:
No, that's the *second* round of unescaped-left-brace situations. The *first* round -- scheduled for 5.26 in May -- concerns this in pod/perldiag.pod: ##### This situation is the subject of this RT. Thank you very much. -- |
From @LeontOn Tue, Jan 10, 2017 at 8:58 PM, James E Keenan via RT <
I see, I had concluded from the context that it was the other part. It does Leon |
From @khwilliamsonOn 01/10/2017 03:07 PM, Leon Timmermans wrote:
As I've said, it has been my expectation that we would end up reverting What I think should be done is revert this, and in early 5.27, make the |
From @xsawyerxOn 04/06/2017 02:41 AM, Leon Timmermans wrote:
I agree. This leaves us little room but to simply revert this and manage |
From @khwilliamsonOn 4/6/2017 11:52 AM, Sawyer X wrote:
I have sent an email to the maintainers, asking for when they plan to |
From @khwilliamsonOn 04/06/2017 01:05 PM, Karl Williamson wrote:
I already received a response, which was effectively, "whenever I can |
From @khwilliamsonReopened |
|
@khwilliamson - Status changed from 'rejected' to 'open' |
From @khwilliamsonOn 04/06/2017 02:36 PM, Karl Williamson via RT wrote:
Think about the following a little before doing a knee-jerk "No". What if we put in a kludge so that it remained fatal except when /\${[^\}]*}/ Thus, when we are about to die, we check if the pattern attempting to be It is the one way I can think of that keeps us from being held hostage What are the downsides? 1. It is inelegant a. to be able to say \w{something} |
From @khwilliamsonOn 04/06/2017 03:51 PM, Karl Williamson wrote:
I now offer a somewhat less kludgy idea. Change the code so that at the This is more general than just the autoconf pattern, and so there are Besides tests and pod, the only changes needed would be to insert these |
From @cpansproutOn Thu, 06 Apr 2017 15:55:14 -0700, public@khwilliamson.com wrote:
Much better. “Unescaped left brace may conflict with future pattern syntax” or suchlike.
-- Father Chrysostomos |
From @xsawyerxOn 04/07/2017 02:46 AM, Father Chrysostomos via RT wrote:
Not a bad idea, but we're pretty far in the cycle to test this change |
From @kentfredricOn 8 April 2017 at 06:07, Sawyer X <xsawyerx@gmail.com> wrote:
Yeah, the cleverer the solution, the more time we need to make sure we Which in practice means this late, without additional test cycles, -- KENTNL - https://metacpan.org/author/KENTNL |
From @khwilliamsonOn 04/07/2017 12:21 PM, Kent Fredric wrote:
I think people should be aware of the full consequences of this. It is not just a simple matter of reverting this patch. Code has If I change the patch to just silently continue, the other code fails to The solution that requires the least cleverness is my original kludge, /\${[^\}]*}/ non-fatal. It's trivial to do this, and keep it to a single warning |
From @xsawyerxOn 04/09/2017 04:51 AM, Karl Williamson wrote:
To be honest, I'm a bit concerned about introducing this kludge. The * Is there a chance we would misrecognize[1] the context in XS code or (If we can move the warning to 5.32, why would the deprecation warning If we go with the kludge, at the very least it would require having [1] This is a real word, I checked. |
From @khwilliamsonOn 04/09/2017 12:01 PM, Sawyer X wrote:
I don't think you understand the proposal. The goal is to work around Any other code anywhere that had that exact same pattern would also warn "Misrecognizing" something other than this pattern as this pattern isn't "Misrecognizing" this pattern for something else would cause a problem
I don't understand the above at all. I wonder if '32' is a typo. My
I mostly agree, and I thought that was already in the plans because of
Don't misunderestimate. |
From @iabyn
+1 -- |
From @khwilliamsonOn 04/10/2017 05:21 AM, Dave Mitchell wrote:
To show how much code is involved in doing this, a minimal patch is |
From @khwilliamson0004-minimal-lbrace-patch.patchFrom f676c52876b6e6ccb34c9410a6557343356d9877 Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Mon, 10 Apr 2017 11:12:53 -0600
Subject: [PATCH 4/4] minimal lbrace patch
---
regcomp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/regcomp.c b/regcomp.c
index 810c457..27a77f3 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -13387,8 +13387,13 @@ S_regatom(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth)
* literal string, or when it's the first thing after
* something like "\b" */
if (len || (p > RExC_start && isALPHA_A(*(p -1)))) {
+ if (memNEs(RExC_start, RExC_end - RExC_start, "\\${[^\\}]*}")) {
RExC_parse = p + 1;
vFAIL("Unescaped left brace in regex is illegal here");
+ }
+ if (PASS2) {
+ ckWARNregdep(p + 1, "Unescaped left brace in regex is deprecated here (and will be fatal in Perl 5.30), passed through");
+ }
}
goto normal_default;
case '}':
--
2.7.4
|
From @xsawyerxOn 04/10/2017 06:20 AM, Karl Williamson wrote:
Fair enough. (I have read dutifully, I just don't have much to add and I don't want
I misread when you wrote 5.30 above. Added another stable and got 32.
We had already released another dev for the @INC change. Several have
<3 |
From @jkeenanOn Wed, 12 Apr 2017 09:06:08 GMT, xsawyerx@gmail.com wrote:
[snip]
Some data ... I did a local checkout of the smoke-me/khw-lbrace branch and yesterday ran it through a "CPAN river" test similar to what I have been reporting on perl.qa for the no-dot-by-default-in-@INC changes. That is, I * set PERL_USE_UNSAFE_INC to 1 (because we're not interested in no-dot failures for the purpose of this branch); * created a list of all CPAN distributions in dependency order; * selected a subset of that list -- in this case, the top 5000 entries; * ran that list through a single 'cpanm' call. Analysis of the 'cpanm' build.log showed no failures due to "Unescaped left brace in regex". To be sure, there are limitations to this approach. A distro which depends on a distro which fails to install for reasons other than the one under consideration cannot be tested and so may still suffer from the problem under consideration. Data available. Thank you very much. -- |
From @iabynOn Mon, Apr 10, 2017 at 11:16:06AM -0600, Karl Williamson wrote:
I *think* we're agreed that this should happen. Will you apply this now? -- |
From @cpansproutOn Sun, 16 Apr 2017 13:23:50 -0700, davem wrote:
In case this helps, it gets a +1 from me, too. -- Father Chrysostomos |
From @khwilliamsonOn 04/16/2017 02:40 PM, Father Chrysostomos via RT wrote:
I haven't heard a clear go-ahead from the pumpking. |
From @LeontOn Sun, Apr 9, 2017 at 4:51 AM, Karl Williamson <public@khwilliamson.com> wrote:
I do think this provides a lesson for the future. It's generally a Quite frankly, if we can miss a target the size of autoconf, one Leon |
From @jkeenanOn 04/17/2017 10:18 AM, Leon Timmermans wrote:
Our primary codebase for testing is the core distribution's internal Our secondary codebase for testing is CPAN. Up until now, those have generally been sufficient. However, going That tertiary codebase would be the "important" executables written in Off the top of my head, I'd say such software distributions would include: Linux: BSD: We would have to work with the people identified as "Perl maintainers" Thank you very much. |
From @xsawyerxOn 04/17/2017 05:52 AM, Karl Williamson wrote:
Sorry. +1! |
From @haargOn Mon, Apr 17, 2017 at 4:18 PM, Leon Timmermans <fawaka@gmail.com> wrote:
Another thing we missed is the go compiler: |
From @khwilliamsonOn 4/17/2017 11:04 AM, Sawyer X wrote:
Now pushed as 7335cb8 |
From @khwilliamsonOn 4/17/2017 8:18 AM, Leon Timmermans wrote:
This is a worthwhile goal, but not really achievable. Such changes |
|
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#130497 (status was 'resolved')
Searchable as RT130497$
The text was updated successfully, but these errors were encountered: