Skip to content

[FIX] file upload broken when running in subdirectory https://github.com…#7395

Merged
rodrigok merged 3 commits intoRocketChat:developfrom
ryoshimizu:fix-file-upload-broken-when-running-in-subdir-6679
Jul 14, 2017
Merged

[FIX] file upload broken when running in subdirectory https://github.com…#7395
rodrigok merged 3 commits intoRocketChat:developfrom
ryoshimizu:fix-file-upload-broken-when-running-in-subdir-6679

Conversation

@ryoshimizu
Copy link
Copy Markdown
Contributor

…//issues/6679

@RocketChat/core

Closes #6679

File upload using jalik:ufs was broken due to the following reasons:

  1. rocketchat-cors/cors.js handler ignoring ROOT_URL_PATH_PREFIX when skipping UFS stores path
  2. rocketchat-file-upload/server/lib/requests.js igoring ROOT_URL_PATH_PREFIX when getting file-upload path
  3. packages/rocketchat-lib/client/lib/settings.js should check for the location.origin + url path prefix for the case when rocketchat is run off a subdir
  4. packages/rocketchat-lib/lib/startup/settingsOnLoadSiteUrl.js dropping the url path prefix when Site_Url is set

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 4, 2017

CLA assistant check
All committers have signed the CLA.

@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@RocketChat RocketChat deleted a comment Jul 4, 2017
@geekgonecrazy
Copy link
Copy Markdown
Contributor

In other places in the code we tend to make use of the Meteor.absoluteUrl() to fix this. Should we try and make use of this? That way the user only has to properly set their ROOT_URL and everything will work properly regardless of subfolder

@ryoshimizu
Copy link
Copy Markdown
Contributor Author

ryoshimizu commented Jul 5, 2017

hi @geekgonecrazy, thanks for the feedback.

we cannot make use of Meteor.absoluteUrl() here because incoming request objects url attribute is a relative URL
https://nodejs.org/api/http.html#http_message_url

and the webapp.handler method is making a decision about whether to handle the request or not based on that attribute

a separate but related issue was that ROOT_URL is not being properly set in the Set_SiteUrl callback, which will result in inconsistent behavior from the Meteor.absoluteUrl() method

@ryoshimizu ryoshimizu changed the title [FIX] file upload issue when running in subdirectory https://github.com… [FIX] file upload broken when running in subdirectory https://github.com… Jul 6, 2017
@ryoshimizu
Copy link
Copy Markdown
Contributor Author

ryoshimizu commented Jul 7, 2017

@geekgonecrazy @rodrigok would you kindly review the changes? Idealy I'd like to have this in the next bugfix release. This should also resolve a lot of issues with folks using a reverse proxy setup running in a subdir

@rodrigok rodrigok added this to the 0.57.3 milestone Jul 14, 2017
@rodrigok rodrigok merged commit 2fec4d8 into RocketChat:develop Jul 14, 2017
@ryoshimizu ryoshimizu deleted the fix-file-upload-broken-when-running-in-subdir-6679 branch July 17, 2017 23:51
@simnv
Copy link
Copy Markdown

simnv commented Jul 20, 2017

Applied this patch to current version - 0.57.2, and it doesn't work.
It only started to work when I've changed this line:
https://github.com/RocketChat/Rocket.Chat/pull/7395/files#diff-d5664ca3caa8062001b0dbddf9679f7cR10
Like this:
WebApp.connectHandlers.use('/file-upload/', Meteor.bindEnvironment(function(req, res, next) {

@ryoshimizu
Copy link
Copy Markdown
Contributor Author

@simnv i think you just dont have your root_url set. I dont see how that change would fix things as Meteor.bindenvironment just makes meteor variables accessible within that scope: it still doesnt change the fact that the /file-upload/ path is missing root_url for its pattern matching

rodrigok added a commit that referenced this pull request Aug 8, 2017
@simnv
Copy link
Copy Markdown

simnv commented Aug 23, 2017

@ryoshimizu Tried with the new version, result is the same. I do have ROOT_URL set:
export ROOT_URL=https://site/rocketchat/
But unless I change the rocketchat_file-upload.js like in my previous comment, uploaded files are not accessible.

@Darkneon
Copy link
Copy Markdown
Contributor

Darkneon commented Aug 23, 2017

@simnv , in a browser console, what are your values for __meteor_runtime_config__.ROOT_URL and __meteor_runtime_config__.ROOT_URL_PATH_PREFIX and what is the src for the image that is inaccessible?

@simnv
Copy link
Copy Markdown

simnv commented Aug 28, 2017

@Darkneon, it's different for rocketchat electron application and for rocketchat in browser.

Electron:
image

Browser:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File upload problem

7 participants