Fix JsonRpc service name #5196

Closed
wants to merge 10 commits into
from

Projects

None yet

4 participants

@blanchonvincent

Json Rpc service name can begin with a "_"

@Ocramius
Zend Framework 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

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

@Ocramius
Zend Framework member

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

@blanchonvincent

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

@blanchonvincent

@Ocramius updated! thank you

@samsonasik

@blanchonvincent travis failure

@blanchonvincent

fixed, thank you @samsonasik

@Maks3w Maks3w commented on an outdated diff Oct 3, 2013
library/Zend/Json/Server/Smd/Service.php
*/
- protected $nameRegex = '/^[a-z][a-z0-9.\\\\_]+$/i';
+ protected $nameRegex = '/^(?!^rpc)[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\.\\\]*$/i';
@Maks3w
Maks3w Oct 3, 2013

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

@Maks3w
Zend Framework member

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

@Maks3w
Zend Framework member

@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
Zend Framework member

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

@blanchonvincent

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

@Maks3w
Zend Framework member

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

@blanchonvincent

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

@Maks3w
Zend Framework member

Good catch @blanchonvincent

@Maks3w
Zend Framework member

@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!

@Maks3w Maks3w was assigned Oct 16, 2013
@Maks3w Maks3w closed this Oct 20, 2013
@gianarb gianarb pushed a commit to zendframework/zend-json that referenced this pull request May 15, 2015
@Maks3w Maks3w Merge pull request zendframework/zendframework#5196 d1a2e86
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment