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: use os.tmpdir() to safely store _karma_webpack_ #279

Merged
merged 1 commit into from
Dec 13, 2017
Merged

fix: use os.tmpdir() to safely store _karma_webpack_ #279

merged 1 commit into from
Dec 13, 2017

Conversation

8bitDesigner
Copy link
Contributor

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
From #230:

Any plugin trying to write to assets folder will fail because karma webpack rewrites webpackOptions.output like so.

webpackOptions.output.path = '/_karma_webpack_/' + indexPath;
webpackOptions.output.publicPath = '/_karma_webpack_/' + publicPath;

Since that path /karma_webpack/... has never existed the plugin which tries to write to asset folder will fail with ENOENT can't open file error.

What is the new behavior?
Instead of using a _karma_webpack_ directory in root, it the plugin now uses a _karma_webpack_ directory from the system's temp folder.

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the following...

  • Impact:
  • Migration path for existing applications:
  • Github Issue(s) this is regarding:

Other information:

@jsf-clabot
Copy link

jsf-clabot commented Nov 30, 2017

CLA assistant check
All committers have signed the CLA.

@michael-ciniawsky michael-ciniawsky changed the title Use os.tmpdir() to safely store _karma_webpack_ fix: use os.tmpdir() to safely store _karma_webpack_ Nov 30, 2017
@michael-ciniawsky michael-ciniawsky added this to the 2.0.7 milestone Nov 30, 2017
Copy link
Contributor

@joshwiens joshwiens left a comment

Choose a reason for hiding this comment

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

Remove everything you added under ./lib

@8bitDesigner
Copy link
Contributor Author

@d3viant0ne, good catch; just rewrote that commit without the build files in lib.

@michael-ciniawsky michael-ciniawsky changed the title fix: use os.tmpdir() to safely store _karma_webpack_ fix: use os.tmpdir() to safely store _karma_webpack_ Dec 1, 2017
Copy link
Contributor

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@Scrum
Copy link
Contributor

Scrum commented Dec 7, 2017

@d3viant0ne approve ?

@bladedancer
Copy link

This is probably user error on my part but with 2.0.6 my tests are running fine. On 2.0.8 I get an error:

14 12 2017 21:04:38.094:DEBUG [middleware:source-files]: Requesting /tmp/_karma_webpack_/3.bundle.js /
14 12 2017 21:04:38.094:DEBUG [middleware:source-files]: Fetching /tmp/_karma_webpack_/3.bundle.js
14 12 2017 21:04:38.094:WARN [web-server]: 404: /tmp/_karma_webpack_/3.bundle.js

Code that seems to be triggering this is react-intl-loader:

const locales = {
	en: () => require('react-intl-loader?locale=en!./en.json'),
	es: () => require('react-intl-loader?locale=es!./es.json')
};

I'll dig into it more but if there's anything obvious that stands out then that would be a great help.

thanks

@Scrum
Copy link
Contributor

Scrum commented Dec 15, 2017

@bladedancer Hi, create new issue pleas. Thanks!

@sechel
Copy link

sechel commented Jul 30, 2018

My test suite also breaks with this PR. The problem is that I have an elaborate webpack config that builds other stuff alongside the karma test bundle. If I run integration tests that depend on those other bundles I could access them under <test-server-name>:port/_karma_webpack_/.... This is not possible anymore after this PR because now all resources are served from some not-guessable /var/tmp/<some-random-stuff>./// location.

@8bitDesigner
Copy link
Contributor Author

Hey mate, you should be able to get a reference to that folder using the node os.tmpdir() method, which is a more secure way of doing what you're trying to do. The former behaviour had karma-webpack writing to the system's root folder, which isn't allowed by most system configurations.

@sechel
Copy link

sechel commented Jul 30, 2018

Yes, you are right. I ended up using the value __webpack_public_path__ that is exposed by webpack and contains this path. This solves most of my problems I'm having with this change but not all. Unfortunately some of my tests, i.e., test driving OAuth2 consent flow, require a fixed URI. Here is the question: Why is the publicPath equal to the physical path? Can't one just use /_karma_webpack_/ as public path and the tmp dir as output.path?

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.

8 participants