s25g5d4 commented 6 years ago

Hi there, it's me again. As I metioned last time I was poking around the headers returned by node-ecstatic, I noticed that the CORS-related headers were set without any option provided by me. And even if I turn the option off explicitly, they still exist. It is not hard to find the problem, but I'm suprised that the problem exists for such a long time.

I also add a test, and hopefully that helps. I don't have much experience on writing tests.

Thanks for reviewing.

jfhbrook commented 6 years ago

Oh neat! My guess is that only a handful of people were using this feature, and I suspect most of them are using the --cors flag.

It's bedtime for me but I can probably look at this tomorrow. Thanks!

codecov-io commented 6 years ago

Codecov Report

Merging #225 into master will decrease coverage by 0.19%. The diff coverage is 0%.

@@            Coverage Diff            @@
##           master     #225     +/-   ##
=========================================
- Coverage   75.86%   75.67%   -0.2%     
=========================================
  Files           9        9             
  Lines         518      518             
  Branches      117      117             
=========================================
- Hits          393      392      -1     
  Misses         45       45             
- Partials       80       81      +1

Impacted Files	Coverage Δ
lib/ecstatic/opts.js	`77.39% <0%> (-0.87%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 29521d1...9b4de71. Read the comment docs.

jfhbrook commented 5 years ago

Closing in favor of #241, which is just this with some minor tweaks and a changelog update.

Thanks a bunch! Sorry this took so long to pull in.

jfhbrook / node-ecstatic

Fix bad parsing on CORS option #225

Codecov Report