Name of distributions
Daniele Varrazzo
both reading the db schema and the META spec, I see that the distribution name is not constrained in any way except that it must have no space. I don't really believe we want specs with a / or a \ or maybe a . in them, don't we? Shouldn't be the case to restrict the names to a more reasonable string?
I'm asking because I'm making some client commands to work both on local files and on downloaded package, so in the command:
pgxn install sha-1.0.0.zip
I have to understand that the last argument is a file... but I can't rule out it's a distribution name :D
So, for the moment I am requiring at least a / in the argument to force working on a local file (and still this could appear in the name as per spec), but I think this request could be relaxed if we stop accepting e.g. the dot in the name. More generally, I wonder if we shouldn't fix a pattern for the distro names, such as (non case sensitive): ^[a-z]([a-z0-9_-]{62}[a-z0-9_])$ (i.e. at most 64 chars, starting with alpha, containing alnum, _ and -, not ending with -).
-- Daniele
David E. Wheeler
> both reading the db schema and the META spec, I see that the distribution
> name is not constrained in any way except that it must have no space. I
> don't really believe we want specs with a / or a \ or maybe a . in them,
> don't we? Shouldn't be the case to restrict the names to a more reasonable
> string?
Well, PostgreSQL doesn't constrain extension names, so I didn't want to constrain distribution names, either. Especially since the whole of PGXN is written so that it needn't be PostgreSQL-specific. If other projects want to create an XN with PGXN's tools, they can.
> I'm asking because I'm making some client commands to work both on local
> files and on downloaded package, so in the command:
>
> pgxn install sha-1.0.0.zip
>
> I have to understand that the last argument is a file... but I can't rule
> out it's a distribution name :D
Well, if -e returns true, assume it's a file. :-)
> So, for the moment I am requiring at least a / in the argument to force
> working on a local file (and still this could appear in the name as per
> spec), but I think this request could be relaxed if we stop accepting e.g.
> the dot in the name. More generally, I wonder if we shouldn't fix a pattern
> for the distro names, such as (non case sensitive):
> ^[a-z]([a-z0-9_-]{62}[a-z0-9_])$ (i.e. at most 64 chars, starting with
> alpha, containing alnum, _ and -, not ending with -).
That's the pattern used for nicknames, IIRC. It's a URI label domain. I dunno. Not really convinced. I try not to put shackles where they aren't absolutely necessary. Maybe I could be convinced, but then it would be less friendly to someone wanting to create PyXN or JSXN, no?
Best,
David
Daniele Varrazzo
On Apr 30, 2011, at 6:31 PM, Daniele Varrazzo wrote:> both reading the db schema and the META spec, I see that the distribution
> name is not constrained in any way except that it must have no space. I
> don't really believe we want specs with a / or a \ or maybe a . in them,
> don't we? Shouldn't be the case to restrict the names to a more reasonable
> string?Well, PostgreSQL doesn't constrain extension names, so I didn't want to constrain distribution names, either. Especially since the whole of PGXN is written so that it needn't be PostgreSQL-specific. If other projects want to create an XN with PGXN's tools, they can.
> I'm asking because I'm making some client commands to work both on local
> files and on downloaded package, so in the command:
>
> pgxn install sha-1.0.0.zip
>
> I have to understand that the last argument is a file... but I can't rule
> out it's a distribution name :DWell, if -e returns true, assume it's a file. :-)
> So, for the moment I am requiring at least a / in the argument to force
> working on a local file (and still this could appear in the name as per
> spec), but I think this request could be relaxed if we stop accepting e.g.
> the dot in the name. More generally, I wonder if we shouldn't fix a pattern
> for the distro names, such as (non case sensitive):
> ^[a-z]([a-z0-9_-]{62}[a-z0-9_])$ (i.e. at most 64 chars, starting with
> alpha, containing alnum, _ and -, not ending with -).That's the pattern used for nicknames, IIRC. It's a URI label domain. I dunno. Not really convinced. I try not to put shackles where they aren't absolutely necessary. Maybe I could be convinced, but then it would be less friendly to someone wanting to create PyXN or JSXN, no?
-- Daniele
David E. Wheeler
> It's not a PG specific need, more a global sanity one. Do you have tests checking that the server works with pathological names? Ones containing / for instance, or non printable chars? Any url broken with a project called \01\02\03...\fe\ff? Is there a constraint for chars > 127? If not, what is their encoding?
I don't, though distribution names are encoded in URLs. So if you uploaded a distribution named "foo/bar", in URLs it would be displayed as "foo%2Fbar". And I have no objection to that, really. Same for unprintable characters. "foo\0bar" gets used in URLs as "foo%00bar". But I think non-printable characters probably ought to be verboten. But I have a hard time thinking that there should be any other limitations. Opinions from other folks?
I should add some tests for sure, though. Just thinking about it now, I suspect that a distribution named "foo/bar" would get a prefix with "foo/bar-1.0.0" in the zip file, so when it was unzipped the directory would be named "foo". Probably ought to fix that.
> I could use the algorithm "if exists it's a file, else it's a spec", but it would result in inconsistent messages: if you mistype a file name you would get an error that a package is not file on the server.
True.
> The other reason is that if somebody is working on a project called 'foo' it may happen for them to have a directory called 'foo' in their project dir and I would like to provide a non ambiguous way to work either on that dir or on an uploaded distribution. With the paranoid hat, it's a potential risk that "an attacker could create a directory called hstore and induce an administrator to blah....". With the script kiddie hat I would upload an extension called '../../etc/passwd' etc.
Well, the first thing any client should do is grab the /index.json from the server. If it then looks at the download API, it can see what it ends in (.zip by default) and then just use that.
So if you type
> pgxn install foo-1.0.0.zip
Then it will install locally if foo-1.0.0.zip is a local file. But if you typed
> pgxn install foo
It wouldn't, because it doesn't end in .zip. That is, I'm assuming that you'd just want to be able to have it work with local zip files, yes?
I can see what you mean, though. "foo-1.0.0.zip" might be considered a distribution name. Hrm. What if we just disallowed a distribution name to end in the suffix specified in the download template? That would allow a distribution named foo-1.0.0.howdy but not foo-1.0.0.zip.
This reminds me: the command
> pgxn install foo
Does that install a distribution that contains an extension named "foo" or does it just look for a distribution named "foo"? I think I would prefer the former. There at least ought to be a way to disambiguate.
> pgxn install extension foo
> pgxn install dist foo
Or something.
> No problem in a charset more extended that that, I just think that no constraint at all is a little bit too much, more thinking about chars that are not valid in the URLs (or have a special meaning such as / and ?) and if all of them are composed the right way everywhere.
Well, maybe we just need to disallow a few characters, like \0 and ?. Thoughts? I think these are okay: / . - _.
Best,
David
Daniele Varrazzo
> This reminds me: the command
>
> > pgxn install foo
>
> Does that install a distribution that contains an extension named "foo" or does it just look for a distribution named "foo"? I think I would prefer the former. There at least ought to be a way to disambiguate.
>
> > pgxn install extension foo
> > pgxn install dist foo
>
> Or something.
foo": I just have to understand what would it mean :) If the ext foo
is in the dist bar together with a dependency, shouldn't it install
the dependency too? Also note that install means "install in the
system", load means "install into a database". Because every ext in a
dist lists its own sql "file", whereas it's likely to be a single
Makefile in the dist - at least now that's what the client looks for,
so I don't know if it's possible at all to compile/install only one of
several possible exts in the dist. Hence an --ext switch could be
probably more useful with the "load" command (which currently does a
"for each ext: install it" and the pattern lends itself to be limited
by user action).
> > No problem in a charset more extended that that, I just think that no constraint at all is a little bit too much, more thinking about chars that are not valid in the URLs (or have a special meaning such as / and ?) and if all of them are composed the right way everywhere.
>
> Well, maybe we just need to disallow a few characters, like \0 and ?. Thoughts? I think these are okay: / . -
should be escaped - you already have found something to tweak because
of them, so it's just easier to disallow them. And, because unicode
opens huge can of worms, either limit to ascii or start testing
everything with a distribution called ☃ (☃ if this message
appears broken).
-- Daniele
David E. Wheeler
> Currently it looks for a dist. I've thought about "pgxn install --ext
> foo": I just have to understand what would it mean :)
This is what I'd always planned for it to be, mainly because that's the way CPAN works. It'd work like this:
pgxn install semver
This would load the "extension" URI template, which would end up requesting /extension/semver.json. Here's an example:
http://master.pgxn.org/extension/semver.json
You can see that it has a "stable" key with the name of the latest stable distribution the semver extension is in, as well as the distribution version and its SHA1. Then you'd have all the data you'd need. Just use the "download" URI template and use the SHA1 to verify it as usual.
If the user wanted to install the latest version, regardless of the releast status, you'd use the value in the "latest" key, here "stable", to decide what to install.
Here's what you see if someone asks to install an extension with no stable version:
http://master.pgxn.org/extension/pgmp.json
Notice that "latest" is "testing" here. Normally you'd ignore that unless the user wanted the latest regardless of the release status, and just go right for the "stable" information. But here there is none, because there's no stable release. So then fall back on "testing" or, if no testing, "unstable" and install that. Perhaps prompt the user to give them the choice whether to install it or not
> If the ext foo
> is in the dist bar together with a dependency, shouldn't it install
> the dependency too?
Yes, I think so.
> Also note that install means "install in the
> system", load means "install into a database". Because every ext in a
> dist lists its own sql "file", whereas it's likely to be a single
> Makefile in the dist - at least now that's what the client looks for,
> so I don't know if it's possible at all to compile/install only one of
> several possible exts in the dist.
No, I'd install the whole dist.
> Hence an --ext switch could be
> probably more useful with the "load" command (which currently does a
> "for each ext: install it" and the pattern lends itself to be limited
> by user action).
I think that's backwards, actually. For the load command, you can *only* specify an extension. I guess you could say --dist foo and it would then load all extensions in the "foo" dist.
>> Well, maybe we just need to disallow a few characters, like \0 and ?. Thoughts? I think these are okay: / . -
>
> I think all the nonprintable, all the whitespaces and I maintain the /
> should be escaped - you already have found something to tweak because
> of them, so it's just easier to disallow them. And, because unicode
> opens huge can of worms, either limit to ascii or start testing
> everything with a distribution called ☃ (☃ if this message
> appears broken).
Ooh, I am so going to release an extension called ☃! That would be rad!
What do others think? Anyone know of a distribution and/or packaging system that allows /, whitespace, non-printable, or non-ASCII characters in its name? Any security issues?
C'mon folks, this can't just be the "David and Daniele Club"! :-)
Thanks,
David
Hitoshi Harada
> On May 1, 2011, at 12:51 PM, Daniele Varrazzo wrote:
> Ooh, I am so going to release an extension called ☃! That would be rad!
>
> What do others think? Anyone know of a distribution and/or packaging system that allows /, whitespace, non-printable, or non-ASCII characters in its name? Any security issues?
I don't think even we Japanese people (, who love non-ASCII issues)
think whitespace/non-ASCII is valid characters in module names :P
--
Hitoshi Harada
David E. Wheeler
>> What do others think? Anyone know of a distribution and/or packaging system that allows /, whitespace, non-printable, or non-ASCII characters in its name? Any security issues?
>
> I don't think even we Japanese people (, who love non-ASCII issues)
> think whitespace/non-ASCII is valid characters in module names :P
Okay. So what if I made the constraint on distribution names this:
/^[-a-z0-9_]/
Any other characters that should be allowed?
Do we need a constraint on extension names, too? I'm thinking not, but am not sure…
Best,
David
Hitoshi Harada
> On May 2, 2011, at 4:20 AM, Hitoshi Harada wrote:
>
>>> What do others think? Anyone know of a distribution and/or packaging system that allows /, whitespace, non-printable, or non-ASCII characters in its name? Any security issues?
>>
>> I don't think even we Japanese people (, who love non-ASCII issues)
>> think whitespace/non-ASCII is valid characters in module names :P
>
> Okay. So what if I made the constraint on distribution names this:
>
> /^[-a-z0-9_]/
>
> Any other characters that should be allowed?
Do you mean /[^-a-z0-9_]/ ? I believe we need more relax, at least
characters in URL should be ok.
> Do we need a constraint on extension names, too? I'm thinking not, but am not sure…
I've not read the upthread all, but I don't prefer to make constraint
on names. We may have common sense.
Regards,
--
Hitoshi Harada
David E. Wheeler
>> Okay. So what if I made the constraint on distribution names this:
>>
>> /^[-a-z0-9_]/
>>
>> Any other characters that should be allowed?
>
> Do you mean /[^-a-z0-9_]/ ?
No, I omitted the $. I meant that the distribution name should match
/^[-a-z0-9_]$/
> I believe we need more relax, at least
> characters in URL should be ok.
Which part of a URL? And not / I assume, yes?
>> Do we need a constraint on extension names, too? I'm thinking not, but am not sure…
>
> I've not read the upthread all, but I don't prefer to make constraint
> on names. We may have common sense.
Yeah, that's what I was thinking.
Thanks,
David
David E. Wheeler
> No, I omitted the $. I meant that the distribution name should match
>
> /^[-a-z0-9_]$/
>
>> I believe we need more relax, at least
>> characters in URL should be ok.
>
> Which part of a URL? And not / I assume, yes?
Here's what I'm thinking:
CREATE DOMAIN namespace AS CITEXT CHECK ( VALUE ~ '^[-a-z0-9_]{2,}$' );
So only ascii letters, numbers, -, and _ would be allowed. Do we need to allow any other characters? Speak now or annoy me in perpetuity!
Best,
David
David E. Wheeler
> Here's what I'm thinking:
>
> CREATE DOMAIN namespace AS CITEXT CHECK ( VALUE ~ '^[-a-z0-9_]{2,}$' );
>
> So only ascii letters, numbers, -, and _ would be allowed. Do we need to allow any other characters? Speak now or annoy me in perpetuity!
Actually, I'm thinking there isn't much reason not to allow ".", is there? I could also see arguments for "," and "+". Yes or no?
Best,
David
Daniele Varrazzo
Apart from having changed the name of the client from pgxn.client to
pgxnclient :) I see no objections in accepting these characters and
leave the distros munging them in case they don't suit their rules.
I've seen "." and "+" used (wouldn't say often), never ",", but it
should have no meaning in url nor in filesystem, so I see no point in
forbidding it.
-- Daniele
David E. Wheeler
>> Actually, I'm thinking there isn't much reason not to allow ".", is there? I could also see arguments for "," and "+". Yes or no?
>
> Apart from having changed the name of the client from pgxn.client to
> pgxnclient :) I see no objections in accepting these characters and
> leave the distros munging them in case they don't suit their rules.
Since you're not (currently) distributing pgxnclient on PGXN, it doesn't really apply. :-)
> I've seen "." and "+" used (wouldn't say often), never ",", but it
> should have no meaning in url nor in filesystem, so I see no point in
> forbidding it.
Not at all worried about URLs; characters there are encoded, including /.
So…hrm. If we allow / and ., it won't make a difference to pgxnclient, will it? So, why not just disallow non-printable characters and space characters, and let anything else in?
Kind of back on the fence on this one.
Best,
David
Daniele Varrazzo
> Not at all worried about URLs; characters there are encoded, including /.
>
> So…hrm. If we allow / and ., it won't make a difference to pgxnclient, will it? So, why not just disallow non-printable characters and space characters, and let anything else in?
You want to create files to be downloaded with a '/' in them? They can
probably be handled with the right amount of escaping, but you won't
gain new friends thanks to them :)
-- Daniele
David E. Wheeler
>> So…hrm. If we allow / and ., it won't make a difference to pgxnclient, will it? So, why not just disallow non-printable characters and space characters, and let anything else in?
>
> You want to create files to be downloaded with a '/' in them? They can
> probably be handled with the right amount of escaping, but you won't
> gain new friends thanks to them :)
Okay, so no unprintable characters or whitespace characters, no /, no \, maybe no :. What's the file separator on VMS?
Anything else?
Best,
David
Joe Abbate
> What's the file separator on VMS?
I presume you mean directory separator, but I'm not sure why you're
asking since PostgreSQL AFAIK doesn't run on VMS (MySQL does have a
port). VMS paths look like this
device:[dir.dir.dir]file.ext
So the directory separator is '.', but ']' also acts like the last '/'
in a POSIX path. Device can really look weird, it's not just a letter
like DOS/Windows. It can something simple like DKA0: or node$dka0: or
even $1$dka0. Also to escape disallowed characters, like a '.' in the
filename (before the extension), a '^' is used. Oh, and '$' is allowed
just about everywhere, even in C variable names.
I'm glad I'm no longer using VMS ...
Joe
David E. Wheeler
> I presume you mean directory separator, but I'm not sure why you're
> asking since PostgreSQL AFAIK doesn't run on VMS (MySQL does have a
> port). VMS paths look like this
>
> device:[dir.dir.dir]file.ext
>
> So the directory separator is '.', but ']' also acts like the last '/'
> in a POSIX path. Device can really look weird, it's not just a letter
> like DOS/Windows. It can something simple like DKA0: or node$dka0: or
> even $1$dka0. Also to escape disallowed characters, like a '.' in the
> filename (before the extension), a '^' is used. Oh, and '$' is allowed
> just about everywhere, even in C variable names.
>
> I'm glad I'm no longer using VMS ...
Damn that sounds convoluted. Think I'll ignore it for now.
Currently, my thinking is to simply disallow:
* whitespace characters
* unprintable characters
* [/\\:]
This I think is the minimum required for sanity. Beyond that, I'm not convinced that disallowing anything else really matters on modern file systems.
amirite?
Best,
David
Daniele Varrazzo
> Currently, my thinking is to simply disallow:
>
> * whitespace characters
> * unprintable characters
> * [/\\:]
>
> This I think is the minimum required for sanity. Beyond that, I'm not convinced that disallowing anything else really matters on modern file systems.
>
> amirite?
+1
-- Daniele
David E. Wheeler
Here's the change that implements this:
https://github.com/pgxn/pgxn-manager/commit/a7785842e2290330ab7fe178cdace0709fc50508
So this will be a *lot* safer for the general case. I was just chatting with depesz on IRC about this, though, and he seems to think that it should be much more limited, like Debian's rules, and more like what I originally suggested: /[-a-z0-9.-+]{2}/. That would certainly be the safest, but I'm not sure it's PGXN's job to prevent names with other characters in them.
I keep going back and forth on this…
Best,
David
David E. Wheeler
> Here's the change that implements this:
>
> https://github.com/pgxn/pgxn-manager/commit/a7785842e2290330ab7fe178cdace0709fc50508
>
> So this will be a *lot* safer for the general case. I was just chatting with depesz on IRC about this, though, and he seems to think that it should be much more limited, like Debian's rules, and more like what I originally suggested: /[-a-z0-9.-+]{2}/. That would certainly be the safest, but I'm not sure it's PGXN's job to prevent names with other characters in them.
>
> I keep going back and forth on this…
I decided to keep it as-is. I think it's reasonable.
And now I'm finding that I probably need to use the same limitation on tags and extension names. The reason? Well, one cannot use / in file names on *nix, *at all*. Someone uploaded an extension with a tag with a slash in it, and this rather broke things.
So I think I'm going to disallow unprintable characters, /, and \ in tag and extension names, as well.
Objections?
Best,
David
Daniele Varrazzo
> So I think I'm going to disallow unprintable characters, /, and \ in tag and extension names, as well.
>
> Objections?
None. As soon as you have implement whatever rule you prefer, tell us
so that I can make the client consistent.
-- Daniele
David E. Wheeler
Finally finished this up and got it deployed last night. Changes:
* Added [version ranges](http://pgxn.org/spec#Version.Range) to the spec. Used for prereqs.
* The "provides" field is now required.
* Tags may no longer contain slashes, backslashes, or control characters, and must be 256 characters or less. Specified via a new [tag data type](http://pgxn.org/spec#Tag).
* Extension names, like distribution names, may no longer contain slashes, backslashes, or control characters, and must be at least 2 characters long. Specified via a new [term data type](http://pgxn.org/spec#Term).
* Added the "conflicts" relationship to the meta spec [`prereqs` field](http://pgxn.org/spec#prereqs).
* The META.json will now be validated by [PGXN::Meta::Validator](http://p3rl.org/PGXN::Meta::Validator). This means that you'll see what the issues are with your META.json as soon as you upload. No more invalid META.json files will be allowed (modulo bugs in the validator, of course).
* As part of the validation, all version strings must be valid [semantic versions](http://semver.org/). PGXN::Manager will no longer try to convert non-semantic versions, as how they should be converted is subject to interpretation.
AS for the validator, it's available on CPAN, and includes a command-line utility:
% cpan PGXN::Meta::Validator
% validate_pgxn_meta META.json
META.json is OK
Will be talking to Daniele and Dickson about how to get that integrated into the other utilities this week or next, at which time you'll be able to do something like `pgxn validate-meta META.json`. Not yet, though.
Best,
David