Add URL normalizePath() filter

[mvalkon]

Adds a filter called normalizePath which implements URL Path
normalization by removing empty path segments and trailing slashes from
the path. This follows the guidance from the Zalando Restful API
Guidelines rule 136.

Resolves #3236

[szuecs]

missing closing " and network backend is not having a path segment.
all: * -> normalizePath() -> "https://backend.example.org";

[szuecs]

NormalizePath -> NormalizePathName

[szuecs]

I think you can return spec, nil and if you have a ptr receiver you don't need to have allocations for the filter:

func NewNormalizePath() filters.Spec { return &normalizePath{} }

func (f *normalizePath) Name() string { return filters.NormalizePathName }

func (f *normalizePath) CreateFilter(config []interface{}) (filters.Filter, error) {
    return f, nil
}

func (f *normalizePath) Request(ctx filters.FilterContext) {
...

[szuecs]

semgrep asks:

did you want path.Join() or filepath.Join()?

[AlexanderYastrebov]

Couple of thoughts:

• Filter name is generic and implies normalization but it only does duplicate slash elimination
• Link to Zalando Guidelines mentions https://en.wikipedia.org/wiki/URI_normalization which mentions RFC 3986 which describes a lot of other normalization methods but does not describe duplicate slash elimination
• There is rfcPath filter that performs escape normalization and also mentions RFC 3986
• This filter will be invoked after routing which means Path-related predicates will not benefit from it.
• There is -rfc-patch-path flag that reuses the same machinery as existing rfcPath at proxy level and thus fixes path before routing.

So I am a bit puzzled here. This filter does not fix routing so maybe we could extend existing rfcPath which also can be enabled globally before routing. On the other hand duplicate slash is not against RFC (see this) so eliminating duplicate slashes is not "RFC"-compliant. Lastly, what problem does it really solve - maybe answering client 404 (in case they sent wrong path that does not match any route) or passing url as is to the backend is a better approach then silently fixing the path.

[mvalkon]

Couple of thoughts:

• Filter name is generic and implies normalization but it only does duplicate slash elimination
• Link to Zalando Guidelines mentions https://en.wikipedia.org/wiki/URI_normalization which mentions RFC 3986 which describes a lot of other normalization methods but does not describe duplicate slash elimination

I agree. The normalization is only normalizing the path following the guidance (see below snippet) from the API guidelines, and I've raised the topic internally to see if we should extend this beyond the scope of this.

We recommend to implement services robust against clients not following this rule. All services should [normalize](https://en.wikipedia.org/wiki/URI_normalization) request paths before processing by removing duplicate and trailing slashes.

• There is rfcPath filter that performs escape normalization and also mentions RFC 3986
• This filter will be invoked after routing which means Path-related predicates will not benefit from it.
• There is -rfc-patch-path flag that reuses the same machinery as existing rfcPath at proxy level and thus fixes path before routing.

I'm happy to extend an existing filter. I have to admit my knowledge of skipper is limited - I'm assuming that this filter would work for all paths if you have a catch-all predicate * defined, but this might be overly simplistic assumption.

So I am a bit puzzled here. This filter does not fix routing so maybe we could extend existing rfcPath which also can be enabled globally before routing. On the other hand duplicate slash is not against RFC (see this) so eliminating duplicate slashes is not "RFC"-compliant. Lastly, what problem does it really solve - maybe answering client 404 (in case they sent wrong path that does not match any route) or passing url as is to the backend is a better approach then silently fixing the path.

While it's not RFC compliant as such, the unfortunate reality is that backend frameworks and server implementations treat trailing slashes in different ways. We had an incident internally recently where an API that previously supported endpoints with trailing slashes stopped doing so after a version upgrade of Spring Boot - the new version no longer treated /foobar and /foobar/ as the same URL. There was of course a client which was incorrectly configured to call /foobar/ which triggered the incident. We clearly guide (see guidelines) that APIs must not specify paths with duplicate or trailing slashes, but we can't control all the client configurations and from there the same guideline recommends server implementations to be robust against poorly configured clients.