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.

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

2.1.0: Wrong file permissions #131

Closed
nilshoerrmann opened this issue Jun 24, 2016 · 21 comments
Closed

2.1.0: Wrong file permissions #131

nilshoerrmann opened this issue Jun 24, 2016 · 21 comments
Assignees
@nitriques
Labels
Bug Fixed in integration
Milestone
2.1.0

Comments

@nilshoerrmann
Copy link
Contributor

nilshoerrmann commented Jun 24, 2016 •
edited

Using the integration branch of this extension, it seems that the cached image files don't respect the write_mode settings from the Symphony config.

Example: Even if I set write_mode to 0777 the resulting image will be saved with 0644.
@nitriques nitriques added the Bug label Jun 24, 2016
@nitriques nitriques added this to the 2.1.0 milestone Jun 24, 2016
@nitriques nitriques self-assigned this Jun 24, 2016
@nitriques
Copy link
Member

Even if I set write_mode to 0777

You should try to avoid this ! 😉

But yeah, it's bug.

@nitriques
Copy link
Member

@nilshoerrmann it seems like it never did... Or I just can't find where it did. Can you confirm that it's a regression ?

@nilshoerrmann
Copy link
Contributor Author

No, I can't confirm that.
It's just not working as I expected.

@nitriques
Copy link
Member

Ok! And what was the permission on the cache folder ?

@nitriques
Copy link
Member

Is the user running the php script owner of the folder ?

@nilshoerrmann
Copy link
Contributor Author

The folder was set to 0777 and the user and owner were different.

@nitriques
Copy link
Member

So I guess you apache umask must be 133 or something like it.

I my setups, all folder and file are own by the php process and 644 is what I want ;)

I guess that it's only because we do not use Symphony's writeFile function (we use GD to save image on disk) but a chmod operation could be made.

@nitriques
Copy link
Member

nitriques commented Jun 29, 2016 •
edited

@nilshoerrmann would you happen to know why the default value for the $perm param in General::uploadFile() method is 0777 ? or maybe @brendo or @michael-e would ?

@michael-e
Copy link
Member

What do you mean by default value? Is there some default in the code that gets overriden by the Symphony configuration?

@nilshoerrmann
Copy link
Contributor Author

I don't understand the question, sorry.

@nitriques
Copy link
Member

What do you mean by default value?

The default value for the parameter in the function. It's never used in the core (we always set a value on the call site).

@michael-e
Copy link
Member

I just guess it is there because otherwise the process would break (if no param is set when calling it). One might as well throw an exception instead… But is this more useful?

@nitriques
Copy link
Member

I would just set the default value to 0644, like we did in writeFile...

@michael-e
Copy link
Member

Default permissions of 0777 are pretty dangerous, generally. So I agree, unless @brendo knows why it was done this way.

@nitriques
Copy link
Member

are pretty dangerous,

Indeed!

@nitriques
Copy link
Member

nitriques commented Jul 7, 2016 •
edited

Since we never had any return from @brendo I git blame'd the thing and it was set to 0777 in rev5 https://github.com/symphonycms/symphony-2/blame/2a85ba629ffb0fe9eaec8f892f442f4734c69232/symphony/lib/toolkit/class.general.php#L1202 (for the complete first commit: symphonycms/symphonycms@7f90fa0)

So I went ahead with changing the default to 0644.

nitriques added a commit to symphonycms/symphonycms that referenced this issue Jul 7, 2016
@nitriques
Change default permission in uploadFile to 0644
255b6dd 
0777 is a dangerous settings and should try to be avoided as a default
value.

Re: symphonycms/jit_image_manipulation#131
nitriques added a commit that referenced this issue Jul 7, 2016
@nitriques
Try to set the proper rights on the cache file
b5584e0 
Cache file should be treated like any other files in Symphony: so we
need to set the proper rights on them after creation.

Fixe #131
@nitriques
Copy link
Member

@nilshoerrmann can you confirm that b5584e0 fixes your problems (i.e. the write_mode is properlly set) ? Thanks!

@nilshoerrmann
Copy link
Contributor Author

I'll check that out later today.

@nitriques
Copy link
Member

Perf, thanks!

@nilshoerrmann
Copy link
Contributor Author

It works as advertised.
Thanks, Nicolas!

@nitriques
Copy link
Member

Awesome!

jensscherbl pushed a commit to symphonycms/symphonycms that referenced this issue May 28, 2017
@nitriques @jensscherbl
Change default permission in uploadFile to 0644
cc83cc8 
0777 is a dangerous settings and should try to be avoided as a default
value.

Re: symphonycms/jit_image_manipulation#131
nitriques added a commit to symphonycms/symphonycms that referenced this issue Jun 16, 2017
@nitriques
Change default permission in uploadFile to 0644
85b4746 
0777 is a dangerous settings and should try to be avoided as a default
value.

Re: symphonycms/jit_image_manipulation#131

Picked from 255b6dd
nitriques added a commit to symphonycms/symphonycms that referenced this issue Jun 16, 2017
@nitriques
Change default permission in uploadFile to 0644
1b19e68 
0777 is a dangerous settings and should try to be avoided as a default
value.

Re: symphonycms/jit_image_manipulation#131

Picked from 255b6dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nitriques nitriques

Labels
Bug Fixed in integration
Projects
None yet
Milestone
2.1.0
Development

No branches or pull requests
3 participants
@nilshoerrmann @michael-e @nitriques