I like this formatting as it looks a lot nicer than the standard query string, however IIS by default will reject that URL before rewriting it and giving it to ACF/Lucee (see http://blogs.iis.net/thomad/iis7-rejecting-urls-containing). I would rather not disable that feature in IIS and wonder if there’s a work around or if I should be doing something else?
The reason for the “+” encoding ending up in the outputted string is due to the ESAPI library that is used, and it’s pretty much the same JAR file that is present in both ACF and Railo/Lucee. Reversing the encoding of the “+” to “%20” would be bad form and so “fixing” this in core isn’t really an option as far as I can tell.
Thanks for the speedy response Justin. I don’t like that it’s not a “bug” but I understand your points. I am still uneasy about enabling double escaping - windows is insecure enough anyways
An alternative migt have been to add a space to the list of non-FU chars in insertQueryVariable() in core/packages/farcry/utils.cfc:614
<cfset var lCharsNotAllowedInFUs = ".,"",',&,@,%,=,/,\" />
As an aside, that function only checks and encodes the value not the key.
If you create a navigation item under root called “My Page”, the system FU is /my-page not /my+page. So farcry is already smart enough to not add +'s there …
Yep, the problem is that the “-” are not encoded/decoded when looking up a friendly URL, they are just used as literal dash characters (check the farFU table and you’ll see what I mean), so we can’t use lCharsNotAllowedInFUs as a fix.
If the ESAPI functions allowed you to encode a space as “%20” instead of “+” then we could look at changing it but I’m not sure that it does.
AFAIK IIS is the only web server that treats “+” characters differently out of the box, so I’d just go with the web.config change if possible.
Justin, on a similar note, I see that in the local CDN service in FC 7.1, you guys are setting the URL path of images using encodeForURL. This is a problem because it causes files with spaces in the name to be replaced with “+”. So if I have “my image.jpg” this creates a URL like “/path/to/my+image.jpg”. + chars are fine for the query string but not filenames. The web server is unable to know if it should load “my+image.jpg” or “my image.jpg” from the file system since + is a valid character for filenames on the disk. There is no way to fix this issue since we have no way to tell Apache that really it should be loading “my image.jpg” and not “my+image.jpg”. The only remedy would be to rename the files with spaces, but that doesn’t prevent users from uploading a new one in the future unless we were to override/modify FarCry’s image/file upload handling. So we’re prevented from going to FarCry 7.1.
I consider this a bug in FarCry 7.1. I noticed this because we have a content type that displays an image in the webtop admin screen and the image was broken. I noticed that the URL for the thumb is going to our 404 page even though the file exists on the disk. There is no benefit to using encodeForURL here as opposed to URLEncodedFormat so I suggest that it be set to use that function to encode the filename instead of encodeForURL.
This sounds a bit like a regression to me. I’m pretty sure we’re supposed to be sanitising any file upload such that filenames are not saved with spaces and/or other inappropriate characters in the filename. That is, FarCry should be renaming the file name to something suitable the first time its written to disk.
Agreed, but I think it is a two part issue. FC should sanitize the filename when its uploaded, but it should not be encoding the file name with “+” for spaces.
Yeah, but that doesn’t help existing data, or data created outside the default, built-in FC upload process, etc. Granted developers who write their own custom upload code should also sanitize the filename on upload.
Well, at the very least if we’re going to leave as-is, it should be documented as a breaking change. We’ll need to run an upgrade script to rename the files on disk and update the filename in the database in order to prevent issues with old data. Obviously, this only works if FC is sanitizing the filename on upload (if it isn’t currently, it should as we’ve discussed).
Spaces are definitely supposed to be replaced with a - when uploaded so we’ll need to review how this keeps being a problem. External processes we don’t have control over but that’s OK.
@seancoyne I agree filenames could probably just be sanitized with URLEncodedFormat, I’ll look at making that change
