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

[BUG] URL validating is oversensitive #78

Closed
ste101 opened this issue Nov 19, 2021 · 5 comments
Closed

[BUG] URL validating is oversensitive #78

ste101 opened this issue Nov 19, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@ste101
Copy link
Contributor

ste101 commented Nov 19, 2021

I tried to set a url propery with a value like this: https://www.test.de
Now I'm getting an validation error because of the FILTER_FLAG_PATH_REQUIRED flag.
I tried the schema validator and Google rich results an they are both happy with
https://www.test.de/
https://www.test.de
www.test.de

Because I have old content with a mix of theese spellings it would be good to delete FILTER_FLAG_PATH_REQUIRED in Classes/Model/Manual/TypeLink.php and remove str_starts_with($link, 'http') some lines below.

@brotkrueml
Copy link
Owner

I assume you are referencing to the Admin Panel? I don't get the problem: The TypeLink is used in Admin Panel's PropertyValueViewHelper where a check of a valid URL is in place beforehand. So this case should not occur.

Please provide an example which illustrates the problem.

@brotkrueml brotkrueml self-assigned this Nov 19, 2021
@brotkrueml brotkrueml added the question Further information is requested label Nov 19, 2021
@ste101
Copy link
Contributor Author

ste101 commented Nov 19, 2021

Sorry, I used the API.

        $manufacturer = TypeFactory::createType('LocalBusiness')->setProperties(
        [
            ...
            'url' => $company['0']->getWww(),
            ...
        ]
        );

Then I got the error called by this line
\sprintf('The given link "%s" ist not a valid URL!', $link),
Then I edited the link to habe the trailing slash and back.
Then I removed the flag and it worked without the slash.

@brotkrueml
Copy link
Owner

Yes, you are right, the check for a URL in https://github.com/brotkrueml/schema/blob/master/Classes/ViewHelpers/AdminPanel/PropertyValueViewHelper.php#L80 is without a path, the check in TypeLink is with the path. This is inconsistent. Will provide a patch.

What version do you need actually? 1 or 2?

@brotkrueml brotkrueml added bug Something isn't working and removed question Further information is requested labels Nov 19, 2021
@ste101
Copy link
Contributor Author

ste101 commented Nov 19, 2021

Please remove FILTER_FLAG_PATH_REQUIRED from TypeLink.php.
Could you also remove the check for the protocol?

@brotkrueml
Copy link
Owner

The FILTER_FLAG_PATH_REQUIRED check will be removed, the protocol not.

Reason: In Admin Panel a URL is linked. Without having a proper URL it can't be linked. That should be no problem for you, as www.example.org and example.org are not linked, but http://www.example.org and http://example.org are.

The error comes from inconsistent URL checking. This is the bug and will be fixed.

brotkrueml added a commit that referenced this issue Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants