search

dotblue / nette-webimages

On-the-fly generated web images for your Nette app
Other
26 stars 18 forks source link

Fix problems with generating images #20

Closed Olicek closed 9 years ago

Olicek commented 9 years ago

I use every project like simlink to /var/www. This PR solve problem with different path to wwwDir and baseDir. I'm not sure, if it will work properly in every other use cases, but I dont know, why shouldn't.

It also fix problem with creating a directory with 777 permission: http://stackoverflow.com/questions/3997641/why-cant-php-create-a-directory-with-777-permissions

vojtech-dobes commented 9 years ago

Can you please split it to 2 commits (relativeUrl & umask)?

milo commented 9 years ago

Imho, using umask() in 3rd party library is a bad practice. @Olicek You can set umask in your application.

Olicek commented 9 years ago

I split it (hope properly, i'm noob with git). @milo Why it is bad practice? It's equivalent to 777 permission, isn't it? Everything what i read about it was connected with permission. Is there any problem hidden for me? thanks.

vincent-io commented 9 years ago

@Olicek umask 000 means that a file will be created with 777 permissions. Whether that's good or bad is too broad to answer, however as a general rule of thumb: when you set your file ownership well 777 should not be needed.

Having said that, I believe @milo's point was about not calling umask() in that particular place.

janedbal commented 9 years ago

@milo :+1:

milo commented 9 years ago

@Olicek Personally I don't want that some library creates files with 777 mask automatically. With umask() in library, you lost control. Libraries should respect system's umask and don't change it. As an application programmer, you can call umask() in your application bootstrap. The effect is the same, library stays clean.

vojtech-dobes commented 9 years ago

Merged without umask.

vojtech-dobes commented 9 years ago

/cc @vvanscherpenseel @milo Can that creation of directory be somehow improved regarding to permissions?

milo commented 9 years ago

@vojtech-dobes I don't think so. Maybe let umask() configurable.

Olicek commented 9 years ago

@milo thanks for the explanation.

If I call umask() in bootstrap, every folder will have 777 permission, won't they? I want 777 just for img folders. I would be for configurable umask(). Can I prepare PR?

vojtech-dobes commented 9 years ago

Okay, lets make it that flexible. By default it should be off. Configuration can look like this:

webimages:
    umask: 0777