Skip Menu |

This queue is for tickets about the Class-XSAccessor CPAN distribution.

Report information
The Basics
Id: 80569
Status: open
Priority: 0/
Queue: Class-XSAccessor

People
Owner: Nobody in particular
Requestors: ribasushi [...] leporine.io
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 1.14
Fixed in: (no value)



Subject: Does not handle \0 containing fields as perl does
In perl it is legal to declare hashes with null-containing keys, e.g. { "foo\0bar" => "bar", "foo\0baz" => "baz" } However Class::XSAccessor seems unaware of this: perl -MDevel::Dwarn -e ' use Class::XSAccessor accessors => { bar => "foo\0bar", baz => "foo\0baz" }; my $x = bless {}; $x->bar(1); $x->baz(2); Dwarn $x' Cheers
On Fri Nov 02 11:08:55 2012, RIBASUSHI wrote: Show quoted text
> In perl it is legal to declare hashes with null-containing keys, e.g. > { "foo\0bar" => "bar", "foo\0baz" => "baz" }
Perfectly valid bug, very bizarre use case... It's hard to fix because perls until recently did not have a way of specifying string (namespace/function-name) length when creating a new XSUB via newXS. On older Perls, one could emulate that somewhat, but my attempts of using newXS(NULL, ...) to create an anon function and then installing it manually have generally invalidated the generated function name in error messages (__ANON__), so I don't have a good solution for any released Perls. Would it be good enough to simply forbid the use of any method names with NULLs? Apart from that, a similar bug pertains to the chosen hash key names (we also suffer from the T_PV XS typemap's strlen() weakness there).
On Fri Nov 02 18:00:19 2012, SMUELLER wrote: Show quoted text
> On Fri Nov 02 11:08:55 2012, RIBASUSHI wrote:
> > In perl it is legal to declare hashes with null-containing keys, e.g. > > { "foo\0bar" => "bar", "foo\0baz" => "baz" }
> > Perfectly valid bug, very bizarre use case... > > It's hard to fix because perls until recently did not have a way of > specifying string (namespace/function-name) length when creating a new > XSUB via newXS. > > On older Perls, one could emulate that somewhat, but my attempts of > using newXS(NULL, ...) to create an anon function and then installing it > manually have generally invalidated the generated function name in error > messages (__ANON__), so I don't have a good solution for any released > Perls. > > Would it be good enough to simply forbid the use of any method names > with NULLs?
I think you misread my bugreport - I was specifically talking about the *hash keys*. Method names containing special chars (or unicode for that matter) can go stuff themselves. Show quoted text
> > Apart from that, a similar bug pertains to the chosen hash key names (we > also suffer from the T_PV XS typemap's strlen() weakness there).
Right - look carefully at my import() call: use Class::XSAccessor accessors => { bar => "foo\0bar", baz => "foo\0baz" }; The method names are normal. The hash-key (field) is the bizarre one.
On Sat Nov 03 04:42:30 2012, RIBASUSHI wrote: Show quoted text
> > Would it be good enough to simply forbid the use of any method names > > with NULLs?
By the way if it is not too expensive it is probably a good idea to do basic linting on *method* names.
On Sat Nov 03 04:43:51 2012, RIBASUSHI wrote: Show quoted text
> On Sat Nov 03 04:42:30 2012, RIBASUSHI wrote:
> > > Would it be good enough to simply forbid the use of any method
names Show quoted text
> > > with NULLs?
> > By the way if it is not too expensive it is probably a good idea to do > basic linting on *method* names.
If expensive, then at "compile" time -- so not an issue. Also, not very expensive. So: Yes, agreed, but: no implied time scale and patches welcome to speed things up. (For the record: This isn't hard just requires changing a fair amount of code. Switch away from using the T_PV typemap in newxs* and use str = SvPV(sv, len) directly to get the length. Then write & use a C function that scans the string for invalid or obscure method names. Even if you don't want to write the code, figuring out what should be valid for method names would be a first step.)
On Sat Nov 03 04:42:30 2012, RIBASUSHI wrote: Show quoted text
> I think you misread my bugreport - I was specifically talking about
the Show quoted text
> *hash keys*. Method names containing special chars (or unicode for
that Show quoted text
> matter) can go stuff themselves.
You're right, I did. I was going to make both work, though. Just that newXS wants a \0 delimited C string for the method name is a bummer for the part that you care less about. Show quoted text
> > Apart from that, a similar bug pertains to the chosen hash key names
(we Show quoted text
> > also suffer from the T_PV XS typemap's strlen() weakness there).
> > Right - look carefully at my import() call: > use Class::XSAccessor accessors => { bar => "foo\0bar", baz =>
"foo\0baz" }; Show quoted text
> The method names are normal. The hash-key (field) is the bizarre one.
Yeah.
Class::XSAccessor 1.15 was just released. It contains a fix for supporting hash keys with NULL / \0 in them. The release does not lint method names. I wasn't quite sure what to allow and what not to, so I punted on it for the time being. I'll leave the ticket open and change it to "wishlist" to indicate the "patches and ideas welcome" nature of the work that left to be done.
On Sun Nov 04 09:31:56 2012, SMUELLER wrote: Show quoted text
> Class::XSAccessor 1.15 was just released. It contains a fix for > supporting hash keys with NULL / \0 in them.
Wow, thanks for the quick turnaround. Show quoted text
> The release does not lint method names. I wasn't quite sure what to allow > and what not to, so I punted on it for the time being. I'll leave the > ticket open and change it to "wishlist" to indicate the "patches and > ideas welcome" nature of the work that left to be done.
Well... given that we are dealing with accessor generation, it makes no sense to me to allow anything other than what perl itself allows. In fact I did just that without thinking twice for CAG[1]. Yes, of course there are things that can be done with $obj->can($exotic_name) and have invocations happen this way... But given the scope of this and similar modules (i.e. accessor *method* generation) I *really* do not see a reason to allow anything outside of qr/\A[A-Z_a-z][0-9A-Z_a-z]*\z/. What was your reason to consider anything more esoteric? [1] https://metacpan.org/source/RIBASUSHI/Class-Accessor-Grouped-0.10006_02/lib/Class/Accessor/Grouped.pm#L80
On Sun Nov 04 11:36:27 2012, RIBASUSHI wrote: Show quoted text
> On Sun Nov 04 09:31:56 2012, SMUELLER wrote:
> > Class::XSAccessor 1.15 was just released. It contains a fix for > > supporting hash keys with NULL / \0 in them.
> > Wow, thanks for the quick turnaround. >
> > The release does not lint method names. I wasn't quite sure what to
> allow
> > and what not to, so I punted on it for the time being. I'll leave
> the
> > ticket open and change it to "wishlist" to indicate the "patches and > > ideas welcome" nature of the work that left to be done.
> > Well... given that we are dealing with accessor generation, it makes > no > sense to me to allow anything other than what perl itself allows. In > fact I did just that without thinking twice for CAG[1]. Yes, of course > there are things that can be done with $obj->can($exotic_name) and > have > invocations happen this way... But given the scope of this and similar > modules (i.e. accessor *method* generation) I *really* do not see a > reason to allow anything outside of qr/\A[A-Z_a-z][0-9A-Z_a-z]*\z/. > > What was your reason to consider anything more esoteric?
Not disallowing anything lets people do clever things. :-) We also have $obj->${\"method"}. (You don’t need to CC me.)
Subject: Re: [rt.cpan.org #80569] Does not handle \0 containing fields as perl does
Date: Mon, 05 Nov 2012 07:18:47 +0100
To: bug-Class-XSAccessor [...] rt.cpan.org
From: Steffen Mueller <smueller [...] cpan.org>
On 11/04/2012 10:56 PM, Father Chrysostomos via RT wrote: Show quoted text
> On Sun Nov 04 11:36:27 2012, RIBASUSHI wrote:
>> Well... given that we are dealing with accessor generation, it makes >> no >> sense to me to allow anything other than what perl itself allows. In >> fact I did just that without thinking twice for CAG[1]. Yes, of course >> there are things that can be done with $obj->can($exotic_name) and >> have >> invocations happen this way... But given the scope of this and similar >> modules (i.e. accessor *method* generation) I *really* do not see a >> reason to allow anything outside of qr/\A[A-Z_a-z][0-9A-Z_a-z]*\z/. >> >> What was your reason to consider anything more esoteric?
> > Not disallowing anything lets people do clever things. :-) > > We also have $obj->${\"method"}.
Ummmmm, are you now making an argument for or against allowing everything? --Steffen
On Mon Nov 05 03:14:17 2012, SMUELLER wrote: Show quoted text
> On 11/04/2012 10:56 PM, Father Chrysostomos via RT wrote:
> > On Sun Nov 04 11:36:27 2012, RIBASUSHI wrote:
> >> Well... given that we are dealing with accessor generation, it makes > >> no > >> sense to me to allow anything other than what perl itself allows. In > >> fact I did just that without thinking twice for CAG[1]. Yes, of course > >> there are things that can be done with $obj->can($exotic_name) and > >> have > >> invocations happen this way... But given the scope of this and similar > >> modules (i.e. accessor *method* generation) I *really* do not see a > >> reason to allow anything outside of qr/\A[A-Z_a-z][0-9A-Z_a-z]*\z/. > >> > >> What was your reason to consider anything more esoteric?
> > > > Not disallowing anything lets people do clever things. :-) > > > > We also have $obj->${\"method"}.
> > Ummmmm, are you now making an argument for or against allowing everything?
For. But notice that the ‘From:’ header has changed. :-)