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

Tube names: add support for colons #210

Conversation

danielbeardsley
Copy link
Contributor

Not sure why they are limited at all except for \n\r\0

Not sure why they are limited at all except for \n\r\0
@kr
Copy link
Member

kr commented Nov 2, 2013

Permitting a fixed list of ok characters is safer and
easier to think about than prohibiting a fixed list of
bad characters.

As for which characters to allow, it's arbitrary, but
we have to draw the line somewhere.

I think there are already plenty of allowed chars.

@xiongchiamiov
Copy link

The motivation behind this change is a transition we're making from gearman to beanstalk. The tube names correspond to worker functions, which, since we're using static methods in PHP, are of the form ClassName::doSomethingInteresting.

We could of course do some string manipulation to dispose of the colons, but it'd be more convenient to be able to use them as-is, and it seemed strange that colons aren't allowed when semi-colons are.

@kr
Copy link
Member

kr commented Nov 4, 2013

And how about some other language that allows vertical bars | in identifiers?
Or at signs @? Or non-ascii unicode letters and numbers? We have to
stop somewhere, might as well be here.

If this is really a problem, rather than patching the server to allow more and
more characters, it would probably be better to have client libraries translate
arbitrary strings into escaped names and be done with it. But it hasn't really
been a problem in the 6 years of beanstalkd's life so far, so I doubt we'll
need it in the future either.

@kr
Copy link
Member

kr commented Nov 4, 2013

I'm going to close this issue to keep the list tidy, but feel free
to continue discussing it here and we can reopen it in the
future if some new information appears.

@kr kr closed this Nov 4, 2013
@extemporalgenome
Copy link

I have no vested interest in making more characters available for names, but for the purpose of discussion, it'd be pretty easy to define protocol-safe tube-names: whitespace, non-printable characters, and non-7bit-ascii is disallowed. That cleanly leaves codes 33-126 (decimal, inclusive), or 0x21-0x7e (hex, inclusive).

@danielbeardsley
Copy link
Contributor Author

I'm with @extemporalgenome -- though I don't care that much anymore. And I'll repeat what I said earlier.

Not sure why they are limited at all except for \n\r\0

@extemporalgenome
Copy link

Not sure why they are limited at all except for \n\r\0

In the interest of forwards-compatibility, it's important to disallow whitespace characters, though, since arguments are separated by spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants