Hi,

Lets say you have a taxonomy (e.g. "Programming Languages") with a term that starts with a . (period), (e.g. ".NET"), and you have a field (let's say "Languages I know", which is a free-tagging autocomplete field.

Now let's say you create a new node that contains this field. You want to enter in ".NET" as a language you know. So you enter in ".N". You would now expect to see ".NET" as a suggestion.
Instead you get an error, because the URL you requested for suggestions received a 403 Forbidden.

The reason for this, is that all directories starting with a period (such as /taxonomy/autocomplete/field_languages/.N), is set to get a 403 Forbidden because of the default supplied .htaccess file (RewriteRule "(^|/)\." - [F])

My suggestion is to prefix the RewriteRule in question with some conditions, so that only existing directories starting with a . (period), receives the 403 Forbidden treatment. i.e.

  RewriteCond %{REQUEST_FILENAME} -f
  RewriteCond %{REQUEST_FILENAME} -d
  RewriteRule "(^|/)\." - [F]

This ensures that "drupal paths" starting with a . (period), is allowed, where as "filesystem paths" aren't.

If desired, I can create a patch and a test-case in regards to this issue.

CommentFileSizeAuthor
#15 taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-15.patch551 bytesseanB
#2 taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-2.patch2.99 KBfangel
#1 taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-1-test-only.patch2 KBfangel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fangel’s picture

fangel CreditAttribution: fangel commented
Status: Active » Needs review
FileSize
taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-1-test-only.patch2 KB

Okay, I've fashioned a proper patch, and updated the UnitTests to highlight to bug.

Here's the updated test-case only

fangel’s picture

fangel CreditAttribution: fangel commented
FileSize
taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-2.patch2.99 KB

And here's the patch + test-case

Status: Needs review » Needs work

The last submitted patch, taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-2.patch, failed testing.

fangel’s picture

fangel CreditAttribution: fangel commented
Status: Needs work » Needs review

#2: taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-2.patch queued for re-testing.

franz’s picture

franz
any
Location Montréal
CreditAttribution: franz commented

@fangel, the test in #1 should've failed, right?

fangel’s picture

fangel CreditAttribution: fangel commented

Yeah, but the auto-testing tools doesn't use rewriting, so hence it doesn't fail in those. It should fail on your on computer if you run the test with clean urls in Apache..

franz’s picture

franz
any
Location Montréal
CreditAttribution: franz commented
Status: Needs review » Needs work
Issue tags: +Needs manual testing, +Needs backport to D7

This also affects D8, so let's re-roll this patch. Marked for manual testing, I can't do because I don't use Apache.

franz’s picture

franz
any
Location Montréal
CreditAttribution: franz commented
Version: 7.x-dev » 8.x-dev
fangel’s picture

fangel CreditAttribution: fangel commented

Well, the patch to fix the issue in itself is only

diff --git a/.htaccess b/.htaccess
index 9494b53..c716642 100644
--- a/.htaccess
+++ b/.htaccess
@@ -75,6 +75,8 @@ DirectoryIndex index.php index.html index.htm
   # If you do not have mod_rewrite installed, you should remove these
   # directories from your webroot or otherwise protect them from being
   # downloaded.
+  RewriteCond %{REQUEST_FILENAME} -f
+  RewriteCond %{REQUEST_FILENAME} -d
   RewriteRule "(^|/)\." - [F]
 
   # If your site can be accessed both with and without the 'www.' prefix, you

which shouldn't need re-rolling (or should be trivial to do so with). The texts however might needs updating.

I don't know when I'll get the time to update the tests, as we're busy with other jobs at my company, and this isn't one with high priority.

John Franklin’s picture

John Franklin CreditAttribution: John Franklin commented

Is this right? Mod_rewrite always makes my head hurt, but this seems to be testing if the request is both a file and a directory, and if so return a 403 FORBIDDEN.

I've done a quick test where I have a site managed with git. Adding in those two rules allows me to fetch the .git/config file. Without the patch http://example.org/.git/config fails.

John Franklin’s picture

John Franklin CreditAttribution: John Franklin commented

A bit more testing and I note the following:

Adding [OR] to the end of the first RewriteCond will return 403 for http://example.org/.git/config, but will return a 404 for http://example.org/.git/no-such-file (where, obviously, no-such-file doesn't exist.) This makes it possible for someone attacking the site to verify a .git repository exists, which is an invitation to try to hack the site via the git repo.

Without the patch any request with a . at the beginning or right after a / (e.g., http://example.org/.git/config, http://example.org/.git/no-such-file and http://example.org/deep/dir/.foo/no-foo-dir, where .foo doesn't exist at all) all return 403.

dcam’s picture

dcam CreditAttribution: dcam commented
Issue summary: View changes

Marked #1661098: Autocomplete and words beginning with dot (.) as a duplicate.

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Version: 8.0.x-dev » 7.x-dev
Issue tags: -Needs backport to D7

The entity reference autocomplete in D8 works just fine with terms that start with a slash because the search string is no longer part of the url, it's in a q query string. Moving back to 7.x.

John Franklin’s picture

John Franklin CreditAttribution: John Franklin commented

Another option is to pass any dot-paths to Drupal, just in case you have some odd corner case where you want a node to have the path http://www.example.org/.git. Something like this might work:

  RewriteRule "(^|/)\." index.php [L]
seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented
Status: Needs work » Needs review
FileSize
taxonomy-Autocomplete-terms-starting-with-period-fails-1232134-15.patch551 bytes

Passing the URLs to Drupal seems to make more sense, provides nice error messages in the users theme and also solves the issue mentioned in #11.

I just created a patch for this.

franz’s picture

franz
any
Location Montréal
CreditAttribution: franz commented

The entity reference autocomplete in D8 works just fine with terms that start with a slash because the search string is no longer part of the url, it's in a q query string. Moving back to 7.x.

Just wanted to clarify, the original report mentions starting with a period, not a slash, does it also work fine?

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

Yes, terms starting with a period also work.

hass’s picture

hass CreditAttribution: hass commented

Why are the terms not URL encoded to prevent such failures? Than a dot becomes %2E and nothing should go wrong with the rewriting.

John Franklin’s picture

John Franklin CreditAttribution: John Franklin commented

@hass, because users can type in the URL by hand with a dot and not encode it.

John Franklin’s picture

John Franklin CreditAttribution: John Franklin commented

Scratch that. I'm thinking of a different issue.