Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more flexible 'index' option for directory handler #10

Merged
merged 4 commits into from Dec 30, 2014

Conversation

@yortus
Copy link
Contributor

yortus commented Dec 3, 2014

This is a simple backwards-compatible extension to the directory handler, allowing the index option to be either a boolean (as before) or a string (new option).

This allows the index file to be configured to be something other than 'index.html', without breaking any previous behaviour.

There's also Joi validation to ensure nothing unsafe can be used.

EDIT: Added units tests.

@kanongil

This comment has been minimized.

Copy link
Member

kanongil commented Dec 4, 2014

This sounds like a nice feature to include. While it won't do much for new deployments, it can simplify migration from other systems.

@hueniverse Would it make sense to allow array of index strings as well, similar to other web servers?

@yortus

This comment has been minimized.

Copy link
Contributor Author

yortus commented Dec 4, 2014

Why the regex?

@kanongil I was just being cautious, because I wasn't sure if allowing an arbitrary string could introduce any security holes.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Dec 4, 2014

Array would be nice. As for security, the result path should go through the same checks as any other content.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Dec 4, 2014

@kanongil Also, no need for the release notes label. That's for issues that are just notes. You can use the changelog system hapi uses for publishing changes if you want (we don't really do it outside hapi).

@yortus

This comment has been minimized.

Copy link
Contributor Author

yortus commented Dec 5, 2014

@kanongil @hueniverse Array support added, with units tests.

I also removed regex from validation, so any string, or array of strings, is permitted for the index option. Do other checks need to be added anywhere?

…Option

Conflicts:
	lib/directory.js
	test/directory.js
@kwo

This comment has been minimized.

Copy link

kwo commented Dec 18, 2014

+1 for this

kanongil added a commit to kanongil/inert that referenced this pull request Dec 30, 2014
more flexible 'index' option for directory handler
kanongil added a commit to kanongil/inert that referenced this pull request Dec 30, 2014
@kanongil kanongil added this to the 2.1.0 milestone Dec 30, 2014
@kanongil kanongil merged commit 2b8ce4e into hapijs:master Dec 30, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@kanongil

This comment has been minimized.

Copy link
Member

kanongil commented Jan 12, 2015

@yortus I have published the 2.1.0 package with this as the only feature. Could you create a PR on hapi with the new version and a documentation update?

@yortus

This comment has been minimized.

Copy link
Contributor Author

yortus commented Jan 18, 2015

@kanongil Done, I think.

PR for hapi: hapijs/hapi#2354 (updated API.md)

PR for hapijs.com: outmoded/hapijs.com#126 (updated tutorial)

package.json in hapi has "inert": "2.x.x", so there's nothing else to change, unless I'm missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.