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

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

Merged
merged 3 commits into from
Jul 14, 2017
Merged

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

merged 3 commits into from
Jul 14, 2017

Conversation

ryoshimizu
Copy link
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

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
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
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
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

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
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
…running-in-subdir-6679

[FIX] file upload broken when running in subdirectory https://github.com…
@simnv
Copy link

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
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

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