8000 EPrints::Update::Static exposes filesystem information · Issue #407 · eprints/eprints · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

EPrints::Update::Static exposes filesystem information #407

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.

Sign up for GitHub

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

Jump to bottom
Open
denics opened this issue Jun 27, 2016 · 10 comments
Open

EPrints::Update::Static exposes filesystem information #407

denics opened this issue Jun 27, 2016 · 10 comments
Labels
enhancement

Comments

@denics
Copy link
Contributor
denics commented Jun 27, 2016 •
edited
Loading

EPrints::Update::Static exposes filesystem when creating auto.js and auto.css this can be avoided by commenting line 270, or adding a test if we are debugging
@denics denics changed the title EPrints::Update::Static expose filesystem information EPrints::Update::Static exposes filesystem information Jun 27, 2016
@mpbraendle
Copy link
Contributor

I don't agree with this commit. The path is useful information, especially for debugging.

@phluid61
Copy link
Contributor

Perhaps it would be better to include only the file path after .../auto/ (e.g. 20_prototype.js)

We have experimental code in our repository that minifies auto.js and auto.css as they're being generated; I'm pretty sure I did a similar thing there (shrinking source filenames), but I don't have access to the source repository right now as I'm out of office.

@phluid61
Copy link
Contributor

I was able to recreate the branch against 3.3: https://github.com/QUTlib/eprints/tree/feature/auto-minifiers

The relevant part of the diff is essentially:

-       print $fh "\n\n\n/* From: $path */\n\n";
+       print $fh "\n/*+$fn*/\n";

@denics
Copy link
Contributor Author
denics commented Jun 28, 2016

Thanks Matthew, this solution is good enough to avoid exposing the filesystem and should be used as a quick fix.
However, a perhaps more elegant solution would be to use the noise parameter like in https://github.com/denics/eprints/commit/a761481a1cd816ed317ec15a6d6994d5dffed009 , tell me what do you think. If you guys agree, I will push the commit here.

@mpbraendle
Copy link
Contributor

I think Matthews solution is fine.
noise or verbose would be only active during the generation of the auto.* files.
However, if you want to debug the JavaScript (in a test or live environment) using the developer tools of the browser, it is good to know from which file the code stems from. Why would I need to always run a generate_static in order to know the origin of the code?

@denics
Copy link
Contributor Author
denics commented Jun 28, 2016

Because you are supposed not to work directly on your production server.
If you have a git environment, you can have your development server with noise = 4 and your production server with noise = 0.

@jesusbagpuss
Copy link
Contributor

Does the 'developer mode' alter the noise?
As I understand things, it causes the auto files to be recompiled with each request. It could also be used to put the full path in if it is on?

@mpbraendle
Copy link
Contributor

Of course your first statement is clear to me. But did you try out noise = 4 once? You wouldn't see the forest anymore because of so many trees.

@denics
Copy link
Contributor Author
denics commented Jun 28, 2016 •
edited
Loading

For example, in my development machine I have a file called archive/{archive}/cfg/cfg.d/0_devel.pl where I set the level of noise. I understand, perhaps setting the loglevel to 4 is too much, maybe 3 would be better, but I strongly encourage the use of a $debug variable than having hardcoded print here and there. especially for highly exposed servers.

789C
@denics
Copy link
Contributor Author
denics commented Jun 30, 2016

Sorry, I just realized I forgot the comment line in my pull https://github.com/denics/eprints/commit/2175f0f597b7c3e40d255afe420219226a920243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@phluid61 @jesusbagpuss @denics @mpbraendle @jiadiyao
0