Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix JsonRpc service name #5196

Closed
wants to merge 10 commits into from
Closed

Fix JsonRpc service name #5196

wants to merge 10 commits into from

Conversation

blanchonvincent
Copy link
Contributor

Json Rpc service name can begin with a "_"

@Ocramius
Copy link
Member

@blanchonvincent was this the reason why __get wasn't recognized?

Additionally, I think all valid identifiers should be discovered (see https://github.com/Ocramius/ProxyManager/blob/0.4.1/src/ProxyManager/Generator/Util/UniqueIdentifierGenerator.php#L30 )

That's the regex for a valid PHP class by the way: http://php.net/manual/en/language.oop5.basic.php

@blanchonvincent
Copy link
Contributor Author

@Ocramius hmm yes and no. Yes because there was a bug, but magic method cannot be seen by the Reflection API.

@Ocramius
Copy link
Member

@blanchonvincent seems to work with http://3v4l.org/lkCHd - is it an old bug?

@blanchonvincent
Copy link
Contributor Author

@Ocramius hmm I'm think I'm wrong about reflection and magic method

@blanchonvincent
Copy link
Contributor Author

@Ocramius updated! thank you

@samsonasik
Copy link
Contributor

@blanchonvincent travis failure

@blanchonvincent
Copy link
Contributor Author

fixed, thank you @samsonasik

*/
protected $nameRegex = '/^[a-z][a-z0-9.\\\\_]+$/i';
protected $nameRegex = '/^(?!^rpc)[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\.\\\]*$/i';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that /i is not longer necessary if you add A-Z

@Maks3w
Copy link
Member

Maks3w commented Oct 3, 2013

There is some definition explaining in detail the allowed characters of a method name?

@blanchonvincent
Copy link
Contributor Author

@Maks3w
Copy link
Member

Maks3w commented Oct 8, 2013

@blanchonvincent I was expecting some kind of explanation about why did you choose that ranges of characters (7f-ff, etc)

Also reading that small paragraph about reserved method names

Method names that begin with the word rpc followed by a period character (U+002E or ASCII 46) are reserved for rpc-internal methods and extensions and MUST NOT be used for anything else.

As far as I understand rpcFoo is valid but rpc.Foo is reserved (notice the period)

@Maks3w
Copy link
Member

Maks3w commented Oct 8, 2013

My point is document enough the regex pattern so we can understand the criteria in the future.

@blanchonvincent
Copy link
Contributor Author

@Maks3w right ! Look the last commit. Thank you.

@Maks3w
Copy link
Member

Maks3w commented Oct 16, 2013

@blanchonvincent The PR looks ok for me. Just a last thing. Why backslash is allowed?

@blanchonvincent
Copy link
Contributor Author

@Maks3w I d'ont want to make a BC :/

@Maks3w
Copy link
Member

Maks3w commented Oct 16, 2013

Good catch @blanchonvincent

@Maks3w
Copy link
Member

Maks3w commented Oct 16, 2013

@blanchonvincent Can you remove the trailing spaces which are making Travis-CI to fail?

  1. ZendTest/Json/Server/Smd/ServiceTest.php (trailing_spaces)
    Coding standards check failed!

@ghost ghost assigned Maks3w Oct 16, 2013
@blanchonvincent
Copy link
Contributor Author

@Maks3w done

Maks3w added a commit that referenced this pull request Oct 20, 2013
@Maks3w Maks3w closed this Oct 20, 2013
gianarb pushed a commit to zendframework/zend-json that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants