Skip to content

Fix redirect - #273

Merged
nicola merged 4 commits into
masterfrom
fix-redirect
Mar 5, 2016
Merged

Fix redirect#273
nicola merged 4 commits into
masterfrom
fix-redirect

Conversation

@deiu

@deiu deiu commented Mar 4, 2016

Copy link
Copy Markdown

Redirect /foo to /foo/ if missing slash and the resource is a container.

Comment thread lib/handlers/get.js Outdated

// Redirect to container with / if missing
if (container && path.lastIndexOf('/') !== path.length - 1) {
debug(contentType)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add some details to this log?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, forgot to remove that debug

@deiu

deiu commented Mar 4, 2016

Copy link
Copy Markdown
Author

Hold on, removing debugs.

[Update] done!

@nicola

nicola commented Mar 4, 2016

Copy link
Copy Markdown
Contributor

Sorry I just tried out the new feature from github that lets you merge the master into the PR so that the PR is up to date

Comment thread lib/ldp.js
// Just return, since resource exists
if (!includeBody) {
return callback(null, stats)
return callback(null, stats, contentType, stats.isDirectory())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we pass stats there is no need to pass stats.isDirectory()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also when is the contentType used?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem occurs later on, since otherwise container will be undefined. This is the relevant line: https://github.com/linkeddata/ldnode/blob/fix-redirect/lib/handlers/get.js#L64

nicola added a commit that referenced this pull request Mar 5, 2016
@nicola
nicola merged commit a44e320 into master Mar 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants