Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bad .perl for paths with pipe characters #4877

Closed
p6rt opened this issue Dec 16, 2015 · 10 comments
Closed

bad .perl for paths with pipe characters #4877

p6rt opened this issue Dec 16, 2015 · 10 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Dec 16, 2015

Migrated from rt.perl.org#126935 (status was 'resolved')

Searchable as RT126935$

@p6rt
Copy link
Author

p6rt commented Dec 16, 2015

From zefram@fysh.org

The .perl method for IO​::Path objects produces faulty output​:

"/foo|bar".IO.perl
q|/foo|bar|.IO(​:SPEC(IO​::Spec​::Unix))
"/foo|bar".IO.perl.EVAL
===SORRY!=== Error while compiling /home/zefram/usr/lucifex/on_guile/EVAL_2
Two terms in a row
at /home/zefram/usr/lucifex/on_guile/EVAL_2​:1
------> q|/foo|^bar|.IO(​:SPEC(IO​::Spec​::Unix))
  expecting any of​:
  infix
  infix stopper
  statement end
  statement modifier
  statement modifier loop

-zefram

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From @lizmat

On 16 Dec 2015, at 12​:31, Zefram (via RT) <perl6-bugs-followup@​perl.org> wrote​:

# New Ticket Created by Zefram
# Please include the string​: [perl #​126935]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl6/Ticket/Display.html?id=126935 >

The .perl method for IO​::Path objects produces faulty output​:

"/foo|bar".IO.perl
q|/foo|bar|.IO(​:SPEC(IO​::Spec​::Unix))
"/foo|bar".IO.perl.EVAL
===SORRY!=== Error while compiling /home/zefram/usr/lucifex/on_guile/EVAL_2
Two terms in a row
at /home/zefram/usr/lucifex/on_guile/EVAL_2​:1
------> q|/foo|^bar|.IO(​:SPEC(IO​::Spec​::Unix))
expecting any of​:
infix
infix stopper
statement end
statement modifier
statement modifier loop

$ 6 'say "/foo|\\bar".IO.perl.EVAL'
"/foo|\bar".IO

Fixed with 8d50dabfa9a3b690b18a , test added with e41c6617855b61678544f9 , can be closed.

Liz

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From zefram@fysh.org

Elizabeth Mattijsen via RT wrote​:

Fixed with 8d50dabfa9a3b690b18a

Done the hard way. Because it lacks most of the refinements of
Str.perl, it looks like it might still have bugs that Str.perl avoids.
For example, leading combining characters will become part of the grapheme
of the opening delimiter, which won't match the closing delimiter if
the matching is grapheme-aware. It is certainly LTA in its treatment
of unprintables. You should (again?) consider calling .perl on the
appropriate path attribute.

The same method also has a similar escaping problem for $!CWD. Here the
easy way doesn't just use a .perl call to escape the string; you can
use .perl for the whole :$!CWD pair. You could profitably do likewise
for the :$!SPEC pair, which doesn't have an escaping problem but does
duplicate serialisation knowledge. I therefore suggest (untested)

  $.is-absolute
  ?? "{$.abspath.perl}.IO({​:$!SPEC.perl})"
  !! "{$.path.perl}.IO({​:$!SPEC.perl}, {​:$!CWD.perl})"

-zefram

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From @lizmat

On 17 Dec 2015, at 16​:16, Zefram <zefram@​fysh.org> wrote​:

Elizabeth Mattijsen via RT wrote​:

Fixed with 8d50dabfa9a3b690b18a

Done the hard way. Because it lacks most of the refinements of
Str.perl, it looks like it might still have bugs that Str.perl avoids.
For example, leading combining characters will become part of the grapheme
of the opening delimiter, which won't match the closing delimiter if
the matching is grapheme-aware. It is certainly LTA in its treatment
of unprintables. You should (again?) consider calling .perl on the
appropriate path attribute.

The same method also has a similar escaping problem for $!CWD. Here the
easy way doesn't just use a .perl call to escape the string; you can
use .perl for the whole :$!CWD pair. You could profitably do likewise
for the :$!SPEC pair, which doesn't have an escaping problem but does
duplicate serialisation knowledge. I therefore suggest (untested)

$.is-absolute
?? "{$.abspath.perl}.IO({​:$!SPEC.perl})"
!! "{$.path.perl}.IO({​:$!SPEC.perl}, {​:$!CWD.perl})"

I see your point about escaping strings in a .perl. This has, however, much wider ramifications that will need to be checked. In any case, Str.perl cannot be used, because it puts double quotes around it. Which would be a set of double quotes too many.

The points about $!SPEC.perlification knowledge living in 2 places and the fact that $!CWD wasn’t .perled, taken into account in 467582dbc9c621fe4bcf0fc363 .

Liz

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From zefram@fysh.org

Elizabeth Mattijsen via RT wrote​:

In any case, Str.perl cannot be used, because it puts double quotes
around it. Which would be a set of double quotes too many.

I think you've misunderstood somewhere. The code that I proposed does
not have a multiple-quotation bug, but what you've committed *does*,
for the CWD part. (I'm not sure what part of my suggestion you think
gets this wrong. None of it introduces any delimiters itself.)

For the main path attribute, calling .perl on the Str gets you one layer
of quotation, which is exactly what you need. For CWD, calling .perl on
the Pair object (as in my code) would again get you the needed single
layer of quotation. What you've committed does "​:CWD<{$!CWD.perl}>",
which totals two layers of quotation. It gets one layer of quotation
from .perl (on the Str value), and then surrounds that in a second
layer of quotation consisting of the literal angle brackets. You could
correct your code by changing the angle brackets to parens, following
the arrangement you used for the SPEC part, "​:SPEC({$!SPEC.perl})".

You got quoting right once already (Str.perl). Stop trying to do the
same job again!

-zefram

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From @lizmat

On 17 Dec 2015, at 17​:31, Zefram <zefram@​fysh.org> wrote​:

Elizabeth Mattijsen via RT wrote​:

In any case, Str.perl cannot be used, because it puts double quotes
around it. Which would be a set of double quotes too many.

I think you've misunderstood somewhere. The code that I proposed does
not have a multiple-quotation bug, but what you've committed *does*,
for the CWD part. (I'm not sure what part of my suggestion you think
gets this wrong. None of it introduces any delimiters itself.)

For the main path attribute, calling .perl on the Str gets you one layer
of quotation, which is exactly what you need. For CWD, calling .perl on
the Pair object (as in my code) would again get you the needed single
layer of quotation. What you've committed does "​:CWD<{$!CWD.perl}>",
which totals two layers of quotation. It gets one layer of quotation
from .perl (on the Str value), and then surrounds that in a second
layer of quotation consisting of the literal angle brackets. You could
correct your code by changing the angle brackets to parens, following
the arrangement you used for the SPEC part, "​:SPEC({$!SPEC.perl})”.

Indeed, it was a thinko, fixed in 12ba3410a13663b801c0

You got quoting right once already (Str.perl). Stop trying to do the
same job again!

Don’t think so​: if you interpolate a Str into a “”, then it calls the .Str method on it, *not* the .perl method. So the path part of the .perl is not .perlified just yet. Looking into that now.

Liz

@p6rt
Copy link
Author

p6rt commented Dec 17, 2015

From zefram@fysh.org

Elizabeth Mattijsen via RT wrote​:

Don't think so​: if you interpolate a Str into a "", then it calls the
.Str method on it, *not* the .perl method.

I'm not sure where you think this is relevant. I was not expecting
implicit .perl calls from any interpolation. The code that I proposed
interpolates the results of explicit .perl calls. My reference to
Str.perl was to say that that's the method that performs quoting
correctly and therefore you should call it (directly or indirectly)
when you require quoting. Indeed, with the Unicode behaviour being as
complicated as it is, any ad hoc attempt to duplicate the quoting logic
is bound to be incomplete.

-zefram

@p6rt
Copy link
Author

p6rt commented May 1, 2017

From @zoffixznet

Looks like the mentioned have been fixed some time ago.

Added another test for leading combiners for good measure, in Raku/roast@1ed18b4319

@p6rt
Copy link
Author

p6rt commented May 1, 2017

@zoffixznet - Status changed from 'open' to 'resolved'

@p6rt p6rt closed this as completed May 1, 2017
@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant