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
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
X-RT-Encrypt: 0
X-RT-Sign: 0
Content-Length: 326
Download (untitled) / with headers
text/plain 326b
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
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 888
Download (untitled) / with headers
text/plain 888b
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).
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 1347
Download (untitled) / with headers
text/plain 1.3k
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.
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-16305-1351932231-1179.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 240
Download (untitled) / with headers
text/plain 240b
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.
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-16305-1351932231-1179.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-16305-1351932231-1179.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-17419-1352036957-822.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 867
Download (untitled) / with headers
text/plain 867b
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.)
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-16305-1351893619-107.80569-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-6061-1351932150-566.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-6061-1352037054-129.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 767
Download (untitled) / with headers
text/plain 767b
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.
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-25884-1352039516-1756.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 382
Download (untitled) / with headers
text/plain 382b
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.
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-25884-1352039516-1756.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-25884-1352039516-1756.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-30463-1352046987-60.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 1139
Download (untitled) / with headers
text/plain 1.1k
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
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-30463-1352046987-60.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-25884-1352039516-1756.80569-0-0 [...] rt.cpan.org> <rt-3.8.HEAD-30463-1352046987-60.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-29264-1352066162-1670.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 1243
Download (untitled) / with headers
text/plain 1.2k
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.)
From smueller [...] cpan.org Mon Nov 5 03: 14:16 2012
X-Ah-From: smueller [...] cpan.org
MIME-Version: 1.0
X-Spam-Status: No, score=-5.586 tagged_above=-99.9 required=10 tests=[AWL=1.314, BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_HI=-5] autolearn=ham
In-Reply-To: <rt-3.8.HEAD-29264-1352066162-704.80569-5-0 [...] rt.cpan.org>
X-Spam-Flag: NO
References: <RT-Ticket-80569 [...] rt.cpan.org> <rt-3.8.HEAD-25884-1352039516-1756.80569-5-0 [...] rt.cpan.org> <rt-3.8.HEAD-30463-1352046987-60.80569-5-0 [...] rt.cpan.org> <rt-3.8.HEAD-29264-1352066162-704.80569-5-0 [...] rt.cpan.org>
X-Virus-Scanned: Debian amavisd-new at bestpractical.com
Message-ID: <50975A47.1050601 [...] cpan.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
X-Ah-Spam-Helo: [192.168.1.10]
X-RT-Original-Encoding: utf-8
X-Spam-Score: -5.586
Authentication-Results: hipster.bestpractical.com (amavisd-new); dkim=pass header.i= [...] booking.com
Received: from localhost (localhost [127.0.0.1]) by hipster.bestpractical.com (Postfix) with ESMTP id B858A24095D for <cpan-bug+Class-XSAccessor [...] hipster.bestpractical.com>; Mon, 5 Nov 2012 03:14:16 -0500 (EST)
Received: from hipster.bestpractical.com ([127.0.0.1]) by localhost (hipster.bestpractical.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yxEm9RKGSKLy for <cpan-bug+Class-XSAccessor [...] hipster.bestpractical.com>; Mon, 5 Nov 2012 03:14:15 -0500 (EST)
Received: from la.mx.develooper.com (x1.develooper.com [207.171.7.70]) by hipster.bestpractical.com (Postfix) with SMTP id BF3642401F9 for <bug-Class-XSAccessor [...] rt.cpan.org>; Mon, 5 Nov 2012 03:14:14 -0500 (EST)
Received: (qmail 18269 invoked by uid 103); 5 Nov 2012 08:14:14 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 5 Nov 2012 08:14:14 -0000
Received: from msx4.booking.com (HELO msx4.booking.com) (91.195.237.19) by 16.mx.develooper.com (qpsmtpd/0.84/v0.84-167-g4ed6cab) with ESMTP; Mon, 05 Nov 2012 00:14:10 -0800
Received: from [62.140.137.109] (port=27311 helo=[192.168.1.10]) by mrx-102.ams4.prod.booking.com with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.80.1) (envelope-from <smueller [...] cpan.org>) id 1TVG1K-0003UG-18 for bug-Class-XSAccessor [...] rt.cpan.org; Mon, 05 Nov 2012 07:18:50 +0100
Delivered-To: cpan-bug+Class-XSAccessor [...] hipster.bestpractical.com
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120912 Thunderbird/15.0.1
Subject: Re: [rt.cpan.org #80569] Does not handle \0 containing fields as perl does
Return-Path: <smueller [...] cpan.org>
Dkim-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=booking.com; s=bk; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:References:Subject:To:MIME-Version:From:Date:Message-ID; bh=EdkVwKPxLhe2BbpjVagI50sqCvNzJ14opkd8t4zcAbY=; b=zmSPuXeMYOiXC1SrxUaBhqKRYbelwExMpT7rT9Ez5QyQ5GTP88CacE+MMQspTWlz3uNdkI9UvFdpp1pa9nOEPh/O1vT6ouSt4rBrg5hv2h0zLecvPPskgTDns5fjQCeLWM1CeJJ4H2Cgwedaw74wozMesMmi8MJgAR+DMm34lWc=;
X-Spam-Check-BY: 16.mx.develooper.com
X-Original-To: cpan-bug+Class-XSAccessor [...] hipster.bestpractical.com
X-RT-Mail-Extension: class-xsaccessor
Date: Mon, 05 Nov 2012 07:18:47 +0100
X-Spam-Level:
X-Ah-RCPT: bug-Class-XSAccessor [...] rt.cpan.org
To: bug-Class-XSAccessor [...] rt.cpan.org
Content-Transfer-Encoding: 7bit
From: Steffen Mueller <smueller [...] cpan.org>
RT-Message-ID: <rt-3.8.HEAD-29189-1352103257-1851.80569-0-0 [...] rt.cpan.org>
Content-Length: 874
Download (untitled) / with headers
text/plain 874b
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
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-29189-1352103257-1851.80569-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <RT-Ticket-80569 [...] rt.cpan.org> <rt-3.8.HEAD-25884-1352039516-1756.80569-5-0 [...] rt.cpan.org> <rt-3.8.HEAD-30463-1352046987-60.80569-5-0 [...] rt.cpan.org> <rt-3.8.HEAD-29264-1352066162-704.80569-5-0 [...] rt.cpan.org> <50975A47.1050601 [...] cpan.org> <rt-3.8.HEAD-29189-1352103257-1851.80569-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-10072-1352124599-1904.80569-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 1009
Download (untitled) / with headers
text/plain 1009b
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. :-)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.