fix: handling special characters #5451
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
rodcoffani
force-pushed
the
fix/special-characters
branch
from
b0effcf to
cc9f253
Compare
rodcoffani
force-pushed
the
fix/special-characters
branch
2 times, most recently
from
35ff8ec to
e7e657e
Compare
imports reordered too
rodcoffani
force-pushed
the
fix/special-characters
branch
from
e7e657e to
95e6938
Compare
rodcoffani
requested review from
diocas,
glpatcern,
jessegeens and
rawe0
as code owners
rodcoffani
force-pushed
the
fix/special-characters
branch
from
95e6938 to
a0f9a30
Compare
rodcoffani
force-pushed
the
fix/special-characters
branch
from
a0f9a30 to
5df898e
Compare
| log.Error().Str("func", "PUTFile").Str("url", finalurl).Str("err", err.Error()).Msg("can't get a new location for a redirection") | ||
| return err | ||
| } | ||
| // FIXME: remove once the URL from 'Location' is properly encoded (with EOS 5.4) |
There was a problem hiding this comment.
so can we remove this now, since the fix is in homedev?
There was a problem hiding this comment.
it depends on how fast the new version of eos is going to production, my suggestion would be to merge this and add the removal to the eos5.4 branch
| encodedParentPath := url.PathEscape(parent.Path) | ||
| encodedMdPath := url.PathEscape(md.Path) | ||
| encodedRelativePath := strings.TrimPrefix(encodedMdPath, encodedParentPath) | ||
| _, err = url.PathUnescape(encodedRelativePath) |
There was a problem hiding this comment.
what's the point of doing this if you don't use the result?
There was a problem hiding this comment.
to check if the URL is still valid after these operations, splitting the unescaping the encodedRelativePath and href in two separate lines.
this intends to have a more precise error message instead of doing all the operations in the same line.
but it shouldn't be a problem to remove this and deal only with the href, err = url.PathUnescape(href) later.
There was a problem hiding this comment.
that might be a bit cleaner, in principle it works anyway so we should never see those messages, and if this ever needs to be debugged because something breaks they can just split the steps then
| return nil, errors.Wrapf(err, "failed to calculate path relative to parent: %v", md.Path) | ||
| } | ||
| href, err = url.JoinPath(hrefBase, relativePath) | ||
| href = url.PathEscape(hrefBase) + encodedRelativePath |
There was a problem hiding this comment.
take a look at https://pkg.go.dev/net/url#URL.ResolveReference for this
Some parts of this PR are a workaround while there isn't a fix in EOS for path encoding.
These can be reverted once there is one.
There is still a known issue with the character % (which should also be solved with the fix)