create endpoint requests with invalid URLs are not rejected

[Theodore Ilie]

When creating an endpoint, it is possible to have an invalid URL by adding a space in "tenant_id."

keystone endpoint-create --service_id 410de5fed7544c90a36ebdb8b38f0cea --publicurl "http://127.0.0.1:8774 /v1.1/\$(tenant_i d)s" --adminurl " http://127.0.0.1:8774 /v1.1/\$(tenant_i d)s" --internalurl "http://127.0.0.1:8774 /v1.1/\$(tenant_i d)s"

It's your own fault if you make an endpoint with a space there, but it would still be nice for Keystone to stop you and give an error message such as, "Cannot create an endpoint with an invalid URL."

Note: This is NOT a duplicate of bug #1098564. This bug is fairly similar to https://bugs.launchpad.net/keystone/+bug/1098564, but that bug report is about not being able to delete the endpoint. I have a patch in progress that adds a test case for deleting the endpoint. My report is about suppressing invalid URLs; the other one is about deleting an endpoint with an invalid URL. These should have separate patches.

[Malini Bhandaru (malini-k-bhandaru)]

Are tenant_ids with a space valid? For that matter, are other characters that can break a url valid?
I think the fix lies at the point where tenant_ids may be created. This invalid url is just a side effect.

[Theodore Ilie (theoilie-ti)]

I think the problem is that tenant_id is a variable/placeholder that is supposed to be replaced with the actual id of a tenant. So when the placeholder is tenant_i d, it cannot be replaced because the program is searching for the exact string "tenant_id" with no space. The same goes for endpoint_id, but I'm not sure exactly what other characters might break a URL.

[Dolph Mathews (dolph)]

I'm not sure how to improve the user feedback here as we're already providing error messages at the time we try to parse URLs... which is the only time we have the data available to do string formatting with them. Is there's a standard way to validate a string is ready to be formatted without actually doing so?

[jiaxi (tjxiter)]

Fix proposed to branch: master
Review: https://review.openstack.org/#/c/200512/

[Steve Martinelli (stevemar)]

wouldn't this be *much* easier to solve by making the clients smarter?

[jiaxi (tjxiter)]

I can solve the bug in the server side throughly without changing the client side.
If I solve the it in the client side.Some code of the server side must be modified.
Because some testcase user uuid string as url in the server client.

And not matter I solve it in the client or server side. The logical is the same.
The code is almost the same.

So I think it's more reasonable and properer to solve it in the server side.

[jiaxi (tjxiter)]

Hello,everyone:

I commited this patch set https://review.openstack.org/#/c/200512/ mainly for suggestion.

keystone.common.validation.parameter_types.py #57 is terribly wrong.
It can't recognize many invalid urls.
Why should we use this regex?

[jiaxi (tjxiter)]

As identity v3 has the same problem. Dolph Mathews strongly suggest me to use this logic to valid v3 endpoint url within this patch set.
I need opinions from more cores and more reviewers about whether should I do it in v3 within this patch set.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/204952

[OpenStack Infra (hudson-openstack)]

Change abandoned by jiaxi (<email address hidden>) on branch: master
Review: https://review.openstack.org/200512
Reason: Submit a new one.This one run into troble.

[OpenStack Infra (hudson-openstack)]

Change abandoned by jiaxi (<email address hidden>) on branch: master
Review: https://review.openstack.org/204952

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/200512
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=312d6c9fd0ab3901cafece0fcb4181490910ea65
Submitter: Jenkins
Branch: master

commit 312d6c9fd0ab3901cafece0fcb4181490910ea65
Author: jiaxi <email address hidden>
Date: Thu Jul 23 04:33:13 2015 -0400

    Reject create endpoint with invalid urls

    The patch is not for general URL validation. It just implements
    a check to make sure any substitutions in the URL will be valid
    when building the catalog.

    The bug also happen in v3. So we have to fix this bug in both
    v2 and v3.

    Closes-Bug: #1471034
    Change-Id: I6728edaae26b140fc19afec6331674c57856985c