WEBrick directory listings take priority over permalinks #6222

MattSturgeon · 2017-07-15T03:45:10Z

pathawks · 2017-07-15T04:19:27Z

So, Webrick doesn't redirect /blog to /blog.html?

MattSturgeon · 2017-07-15T04:29:55Z

That's right. If you remove/modify the /blog/ prefix from the /blog/:title permalink, it works fine. I'm guessing WEBrick only redirects blog to blog.html if a blog directory doesn't actually exist.

MattSturgeon · 2017-07-15T06:05:41Z

Disabling directory indexing by setting :FancyIndexing to false for WEBrick::HTTPServlet::FileHandler (somewhere in here?) may help fix this.

While I know gh-pages probably uses a different http server (maybe nginx or apache), I couldn't reproduce this bug when I tested on github.io.

Using python's http.server module, which shows directory indexes too, I reproduced this.

pathawks · 2017-07-15T06:31:44Z

I wonder if there is some way we could configure one to take priority over the other

MattSturgeon · 2017-07-18T21:20:20Z

Ok, looking into WEBrick::HTTPServlet::FileHandler a little more, and learning some ruby as I go. The directory indexing stuff only happens after the normal GET request handling has failed, so disabling :FancyIndexing won't fix this bug anyway.

It may be possible to extend search_index_files(req, res) to fallback to filename + '.html' after its default check of filename + '/', but this may mess with the path_info magic that set_filename(req, res) is doing, so we may end up having to override that too.

Thinking of something along these lines:

# untested code. TODO: setup jekyll dev environment!
def search_index_file(req, res)

  # First try the default check for index files in the directory
  if file = super
      return file
  end
  
  # Otherwise, try a custom check for directory.html in the parent directory
  
  # Extract basename from the end of res.filename
  path_arr = res.filename.scan(%r{/[^/]*})

  # Get the last non-empty section of the filename
  while basename = path_arr.pop
      break unless basename == '/'
  end

  if file = search_file(req, res, basename + '.html')
      res.filename = path_arr.join
      return file
  end
  
  return nil

end

Jekyll extends WEBrick to serve file.html if file is requested but doesn't exist (see jekyll#3452). This doesn't work if file exists as a directory though. This commit extends WEBrick further so that if file exists as a directory, we first try and serve file/index.html (as normal), but fallback to file.html if no index can be found within the file directory (reported as jekyll#6222). It is slightly hacky, to avoid having to re-implement the entire set_filename method, we instead extend search_index_file and temporarily mutate res.filename. This is all clearly commented. See base class search_index_file implementation: https://github.com/ruby/webrick/blob/3fb6a011d620f1c19fce71bc3447740553e4fa80/lib/webrick/httpservlet/filehandler.rb#L365-L385

Jekyll extends WEBrick to serve file.html if file is requested but doesn't exist (see jekyll#3452). This doesn't work if file exists as a directory though. This commit extends WEBrick further so that if file exists as a directory, we first try and serve file/index.html (as normal), but fallback to file.html if no index can be found within the file directory (reported as jekyll#6222). It is slightly hacky, to avoid having to re-implement the entire set_filename method, we instead extend search_index_file and temporarily mutate res.filename. This is all clearly commented. See base class search_index_file implementation: https://github.com/ruby/webrick/blob/3fb6a011/lib/webrick/httpservlet/filehandler.rb#L356-L363

Jekyll extends WEBrick to serve file.html if file is requested but doesn't exist (see jekyll#3452). This doesn't work if file exists as a directory though. This commit extends WEBrick further so that if file exists as a directory, we first try and serve file/index.html (as normal), but fallback to file.html if no index can be found within the file directory (reported as jekyll#6222). It is slightly hacky, to avoid having to re-implement the entire set_filename method, we instead extend search_index_file and mutate res.filename, reverting the change if it isn't needed. See base class search_index_file implementation: https://github.com/ruby/webrick/blob/3fb6a011/lib/webrick/httpservlet/filehandler.rb#L356-L363

Fix jekyll#6222, related to jekyll#3452 See base class search_index_file implementation: https://github.com/ruby/webrick/blob/3fb6a011/lib/webrick/httpservlet/filehandler.rb#L356-L363

Fix jekyll#6222, related to jekyll#3452 See base class search_file implementation: https://github.com/ruby/webrick/blob/3fb6a011/lib/webrick/httpservlet/filehandler.rb#L365-L385

janpio · 2017-10-21T11:16:06Z

Awesome that this was fixed in 3.6.x, but I think there still might something slightly broken maybe:

(I hope it is ok just to reply to this issue, parts of the PR was so salty that I am a bit scared...)

I have a Jekyll project with permalink: /:path that has a page.md and a page/second-page.md. This should be served as /page and /page/second-page.

In 3.5.2 going to /page showed the directory listing. Now in 3.6.0 this is fixed thanks to this PR (if I understood correctly). Problem is while the page works, /page is still redirected to /page/ (which is different to expected behaviour and also different to how it works on Github pages).

Here is a screenshot of Firefox devtools of the redirection:

Did I find the correct issue connected to this?
What would be an appropriate next step?

pathawks · 2017-10-21T16:22:17Z

@janpio So /page should not show a directory listing, and should not redirect to /page/, but should display /page.html, is that correct?

That seems like a regression, if this changed from 3.5.x to 3.6.0.

Could you open a new issue explaining all of this? This deserves it's own conversation.

janpio · 2017-10-21T16:58:43Z

Almost.

/page should show the content.
In 3.5.x it showed the directly listing after redirecting to /page/.
Since 3.6.0 it shows the correct content but still redirects to /page/. Content good, redirect still bad.

But just now I noticed that /page/second-page does actually render the content that should be at /page, which is of course really bad.

I will create a new issue with repro etc.

pathawks · 2017-10-21T17:13:59Z

What is on the disk, in the _site directory?

If the content is at /page/index.html then it seems like a redirect from /page to /page/ is acceptable. But yes, do open an issue so we can give this the attention it needs.

janpio · 2017-10-21T18:28:17Z

No, content is in page.html. (Only page/index.md should be rendered to /page/index.html, right?)

See this new issue for complete description and reproduction repo:
#6459

MattSturgeon · 2017-10-21T20:22:43Z

Is the issue being reported that Jekyll appends a trailing slash (/) to the URI? Correct content shown but incorrect URI formatting?

…

On Sat, 21 Oct 2017, 19:28 Jan Piotrowski, ***@***.***> wrote: No, content is in page.html. (Only page/index.md should be rendered to /page/index.html, right?) See this new issue for complete description and reproduction repo: #6459 <#6459> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6222 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE0BIsmV6z84A77LPJdRxGxAmNHW8tpfks5sujfbgaJpZM4OY575> .

janpio · 2017-10-21T21:01:50Z

That is one part, yes.

Unfortunately there is another, pages inside the folder show the same content as the page clashing with the folder name. This only started with 3.6.x. See #6459 for a proper bug report with repro etc.

janpio · 2017-10-23T10:13:00Z

To make it less confusing I extracted the both problem (unnecessary redirect, wrong content rendered) into to seperate issues: #6459 + new #6475

We upgraded Jekyll to 2.6.3 (a minor version upgrade from 2.4.3) to fix a security vulnerability, but the entire 2.6.x line has a bug with how URLs are handled in the built-in web server. (Fixed here: jekyll/jekyll#6459 and jekyll/jekyll#6475, originally introduced in the fix to jekyll/jekyll#6222 (comment)). Anyway, 2.8.5 includes a fix and generates an identical `_site` directory on disk, so should be all good.

MattSturgeon changed the title ~~permalink cannot be the same as part of another~~ WEBrick directory listings take priority over permalinks Jul 15, 2017

jekyllbot added the has-pull-request Somebody suggested a solution to fix this issue label Jul 18, 2017

MattSturgeon mentioned this issue Jul 18, 2017

Fix serving files that clash with directories (#6222) #6231

Merged

jekyllbot closed this as completed in #6231 Jul 25, 2017

ashmaroli mentioned this issue Nov 20, 2017

return correct file in dir if dir has same name as file #6569

Merged

Mr0grog mentioned this issue Feb 6, 2019

Fix security vulnerabilities in Rack, FFI, and Jekyll edgi-govdata-archiving/100days#105

Merged

jekyll locked and limited conversation to collaborators Jul 12, 2019

jekyllbot added the frozen-due-to-age label Jul 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WEBrick directory listings take priority over permalinks #6222

WEBrick directory listings take priority over permalinks #6222

MattSturgeon commented Jul 15, 2017 •

edited

Loading

pathawks commented Jul 15, 2017

MattSturgeon commented Jul 15, 2017 •

edited

Loading

MattSturgeon commented Jul 15, 2017

pathawks commented Jul 15, 2017

MattSturgeon commented Jul 18, 2017 •

edited

Loading

janpio commented Oct 21, 2017 •

edited

Loading

pathawks commented Oct 21, 2017

janpio commented Oct 21, 2017

pathawks commented Oct 21, 2017

janpio commented Oct 21, 2017

MattSturgeon commented Oct 21, 2017 via email

janpio commented Oct 21, 2017

janpio commented Oct 23, 2017

WEBrick directory listings take priority over permalinks #6222

WEBrick directory listings take priority over permalinks #6222

Comments

MattSturgeon commented Jul 15, 2017 • edited Loading

My Reproduction Steps

The Output I Wanted

pathawks commented Jul 15, 2017

MattSturgeon commented Jul 15, 2017 • edited Loading

MattSturgeon commented Jul 15, 2017

pathawks commented Jul 15, 2017

MattSturgeon commented Jul 18, 2017 • edited Loading

janpio commented Oct 21, 2017 • edited Loading

pathawks commented Oct 21, 2017

janpio commented Oct 21, 2017

pathawks commented Oct 21, 2017

janpio commented Oct 21, 2017

MattSturgeon commented Oct 21, 2017 via email

janpio commented Oct 21, 2017

janpio commented Oct 23, 2017

MattSturgeon commented Jul 15, 2017 •

edited

Loading

MattSturgeon commented Jul 15, 2017 •

edited

Loading

MattSturgeon commented Jul 18, 2017 •

edited

Loading

janpio commented Oct 21, 2017 •

edited

Loading