Add support for XDG Base Directory Specification #28943

New Issue

Cosme Domínguez Díaz · 2011-10-17T04:42:59+02:00

2011-10-17 04:42:59 +02:00

%%%Please, read:

http://standards.freedesktop.org/basedir-spec/latest/

and some opinions about it

http://stick.gk2.sk/blog/2009/03/the-ugly-duckling-called-xdg_config_home/
http://ploum.net/post/184-cleaning-user-preferences-keeping-user-data
http://ploum.net/post/207-modify-your-application-to-use-xdg-folders

I made two patches that fix this issue: one for config files and the other for cache files.

I have tried them and it seems works well.%%%

%%%Please, read: http://standards.freedesktop.org/basedir-spec/latest/ and some opinions about it http://stick.gk2.sk/blog/2009/03/the-ugly-duckling-called-xdg_config_home/ http://ploum.net/post/184-cleaning-user-preferences-keeping-user-data http://ploum.net/post/207-modify-your-application-to-use-xdg-folders I made two patches that fix this issue: one for config files and the other for cache files. I have tried them and it seems works well.%%%

Cosme Domínguez Díaz commented

2011-10-17 04:42:59 +02:00

Changed status to: 'Open'

Cosme Domínguez Díaz commented

2011-10-17 04:46:00 +02:00

%%%Attached patches%%%

Brecht Van Lommel commented

2011-10-19 01:17:22 +02:00

%%%Changing paths should not be done lightly, but I don't have a good idea yet of the current typical unix conventions here so will not comment on that now. From reading over the second patch though, some comments:

XDG_DOCUMENTS_DIR should not be used on Mac.
It wouldn't say that we use the temporary directory as cache, I don't think it can simply be replaced.
The call to strcat seems like it can give a buffer overflow because maxlen is not checked.%%%

%%%Changing paths should not be done lightly, but I don't have a good idea yet of the current typical unix conventions here so will not comment on that now. From reading over the second patch though, some comments: * XDG_DOCUMENTS_DIR should not be used on Mac. * It wouldn't say that we use the temporary directory as cache, I don't think it can simply be replaced. * The call to strcat seems like it can give a buffer overflow because maxlen is not checked.%%%

Cosme Domínguez Díaz commented

2011-10-19 02:01:58 +02:00

%%%First of all, thanks for replying! ^_^

About your first point:

#if !defined(WIN32)

 const char *xdg_documents_dir = getenv("XDG_DOCUMENTS_DIR");

```
 if (xdg_documents_dir)
```
```
      return xdg_documents_dir;
```

   return getenv("HOME");

XDG_DOCUMENTS_DIR will only be used if exists.

So if MacOS or other Unix systems don't have that enviroment variable then Blender will use the HOME variable as it was before.

"$XDG_CACHE_HOME defines the base directory relative to which user specific non-essential data files should be stored."

Source: http://standards.freedesktop.org/basedir-spec/latest/ar01s03.html

Then, we don't use the temporary directory for store "user specific non-essential data files"?

As far as I know Blender doesn't store any non-essential data relative to the system ( /tmp/ dir). Blender is a user app, no?

I'm no expert and maybe you're right. But since you mention it I do not understand because in the same file (path_util.c) we use about nine times the function strcat...

Anyway, do you recommend some particular function?

I just saw that strncat is more safe than strcat but I don't know if Blender have their own version (maybe BLI_strcat?)%%%

%%%First of all, thanks for replying! ^_^ * About your first point: #if !defined(WIN32) + const char *xdg_documents_dir = getenv("XDG_DOCUMENTS_DIR"); + if (xdg_documents_dir) + return xdg_documents_dir; ``` return getenv("HOME"); ``` XDG_DOCUMENTS_DIR will only be used if exists. So if MacOS or other Unix systems don't have that enviroment variable then Blender will use the HOME variable as it was before. * "$XDG_CACHE_HOME defines the base directory relative to which user specific non-essential data files should be stored." Source: http://standards.freedesktop.org/basedir-spec/latest/ar01s03.html Then, we don't use the temporary directory for store "user specific non-essential data files"? As far as I know Blender doesn't store any non-essential data relative to the system ( /tmp/ dir). Blender is a user app, no? * I'm no expert and maybe you're right. But since you mention it I do not understand because in the same file (path_util.c) we use about nine times the function strcat... Anyway, do you recommend some particular function? I just saw that strncat is more safe than strcat but I don't know if Blender have their own version (maybe BLI_strcat?)%%%

Cosme Domínguez Díaz commented

2011-10-19 10:11:55 +02:00

%%%Update patches to use strncat instead of strcat%%%

Sergey Sharybin commented

2011-10-19 15:25:00 +02:00

%%%Probably supporting XDG can be nice to store configuration files, but i'm not sure how many users of blender who've got this variable set. So it sounds more like moving .blender from home directory to ~/.config. Can't see real benefit from this.
As Brecht, i'm not really agree with changing temporary directory with something inside user home directory. And strncat doesn't solve buffer overflow error:

getenv("HOME") can easily return NULL and in this case you'll have a sigsegv
buffer, returned by getenv("HOME") can easily not contain enough allocated bytes to append /.config there.
It should be done by checkign what getenv returned and storing result in your own buffer. Also not sure, but are you sure paths used by blender and gameengine are the same for all cases?

And don't think it's really a bug and probably better to move discussion to patch tracker or to mailing list/irc.%%%

%%%Probably supporting XDG can be nice to store configuration files, but i'm not sure how many users of blender who've got this variable set. So it sounds more like moving .blender from home directory to ~/.config. Can't see real benefit from this. As Brecht, i'm not really agree with changing temporary directory with something inside user home directory. And strncat doesn't solve buffer overflow error: - getenv("HOME") can easily return NULL and in this case you'll have a sigsegv - buffer, returned by getenv("HOME") can easily not contain enough allocated bytes to append /.config there. It should be done by checkign what getenv returned and storing result in your own buffer. Also not sure, but are you sure paths used by blender and gameengine are the same for all cases? And don't think it's really a bug and probably better to move discussion to patch tracker or to mailing list/irc.%%%

Cosme Domínguez Díaz commented

2011-10-20 00:05:02 +02:00

%%%$XDG_CONFIG_HOME and $XDG_CACHE_HOME variables are set by xdg-user-dirs package that it's installed by default in Debian, Ubuntu, Fedora, OpenSUSE, CentOS, RHEL.... and almost all Linux distributions that have the usually default directories: Downloads, Documents, Images, Videos, Templates and Public.

I think all those are a lot of potential users of Blender.

"As Brecht, i'm not really agree with changing temporary directory with something inside user home directory."

Why? Sorry, maybe you know more about the workings of Blender. But I don't understand why you want to write in the most insecure directory that exists on the system. Please, see - [x] and [2]

Also, TEMP and TMP variables used in Windows point to %USERPROFILE%\AppData\Local\Temp - [x]... so why not save temporary files in ~/.cache/ ?

"Insecure creation of files in /tmp when rendering. (Linked to CVE-2008-1102)" http://projects.blender.org/tracker/?func=detail&group_id=9&aid=18174&atid=264
"CVE-2008-1103 Blender insecure temporary file usage" https://bugzilla.redhat.com/show_bug.cgi?id=444535
"Recognized Environment Variables" http://technet.microsoft.com/en-us/library/dd560744%28WS.10%29.aspx

"It should be done by checking what getenv returned and storing result in your own buffer."

I will try to improve the patches using your suggestions, Sergey.

"Also not sure, but are you sure paths used by blender and gameengine are the same for all cases?"

Thanks, I will check that.

For now I only found this:

$ grep -R getenv * | grep -v .svn
GamePlayer/xembed/blenderplayer-wrapper.c: const char* display = getenv("DISPLAY");
$ grep -R home * | grep -v .svn
GamePlayer/ghost/GPG_ghost.cpp: pathname = "homeuser//";
Ketsji/KX_PythonInit.cpp: * "/home/me/foo.blend" -> "/home/me/scripts"

In source/gameengine dir.

Thanks for all! ^_^%%%

%%%$XDG_CONFIG_HOME and $XDG_CACHE_HOME variables are set by xdg-user-dirs package that it's installed by default in Debian, Ubuntu, Fedora, OpenSUSE, CentOS, RHEL.... and almost all Linux distributions that have the usually default directories: Downloads, Documents, Images, Videos, Templates and Public. I think all those are a lot of potential users of Blender. "As Brecht, i'm not really agree with changing temporary directory with something inside user home directory." Why? Sorry, maybe you know more about the workings of Blender. But I don't understand why you want to write in the most insecure directory that exists on the system. Please, see - [x] and [2] Also, TEMP and TMP variables used in Windows point to %USERPROFILE%\AppData\Local\Temp - [x]... so why not save temporary files in ~/.cache/ ? - [x] "Insecure creation of files in /tmp when rendering. (Linked to CVE-2008-1102)" http://projects.blender.org/tracker/?func=detail&group_id=9&aid=18174&atid=264 - [x] "CVE-2008-1103 Blender insecure temporary file usage" https://bugzilla.redhat.com/show_bug.cgi?id=444535 - [x] "Recognized Environment Variables" http://technet.microsoft.com/en-us/library/dd560744%28WS.10%29.aspx "It should be done by checking what getenv returned and storing result in your own buffer." I will try to improve the patches using your suggestions, Sergey. "Also not sure, but are you sure paths used by blender and gameengine are the same for all cases?" Thanks, I will check that. For now I only found this: $ grep -R getenv * | grep -v .svn GamePlayer/xembed/blenderplayer-wrapper.c: const char* display = getenv("DISPLAY"); $ grep -R home * | grep -v .svn GamePlayer/ghost/GPG_ghost.cpp: pathname = "*home*user//"; Ketsji/KX_PythonInit.cpp: * "/home/me/foo.blend" -> "/home/me/scripts" In source/gameengine dir. Thanks for all! ^_^%%%

Campbell Barton commented

2011-10-25 05:17:54 +02:00

%%%First I think we should agree if we want to use xdg folders at all.

Changing this stuiff should not be taken lightly but many applications are using this standard, in principle I like the goals but we need to make sure this works for us and will not annoy users by moving paths about.

Its worth noting that this came up when we re-arranged paths for 2.5x but was rejected without much discussion.

do we even want this?
if so, work out how to add in the update.
Suggest this first be added as a build option WITH_XDG_PATHS for eg, then we can spend some time to make it work correctly, once its generally working well we can enable by default OR keep off but distrobutions may want to enable.

My main concern here is that the ability to store config in 2 locations adds confusion for users, especially in the time when we move from one to the other. or for users who start with their distrobutions blender version and switch to a build with WITH_XDG_PATHS set differently, for the slight convenience of storing settings in ".config/blender" we get all this extra annoyance :S... its a tradeoff.

Would rather not discuss moving temp dir at the moment and first figure out if we want xdg paths.

Cosme, Regarding you're last post:

GamePlayer/xembed/blenderplayer-wrapper.c --- this file is not maintained or compiled into releases (we should really remove it).
GamePlayer/ghost/GPG_ghost.cpp: pathname = "homeuser*"; --- this is only an example string for printing help text (* are stupid, replaced with /).
Ketsji/KX_PythonInit.cpp: * "/home/me/foo.blend" -> "/home/me/scripts" --- this is a comment.
%%%

%%%First I think we should agree if we want to use xdg folders at all. Changing this stuiff should not be taken lightly but many applications are using this standard, in principle I like the goals but we need to make sure this works for us and will not annoy users by moving paths about. Its worth noting that this came up when we re-arranged paths for 2.5x but was rejected without much discussion. 0) do we even want this? 1) if so, work out how to add in the update. Suggest this first be added as a build option WITH_XDG_PATHS for eg, then we can spend some time to make it work correctly, once its generally working well we can enable by default OR keep off but distrobutions may want to enable. My main concern here is that the ability to store config in 2 locations adds confusion for users, especially in the time when we move from one to the other. or for users who start with their distrobutions blender version and switch to a build with WITH_XDG_PATHS set differently, for the slight convenience of storing settings in ".config/blender" we get all this extra annoyance :S... its a tradeoff. Would rather not discuss moving temp dir at the moment and first figure out if we want xdg paths. Cosme, Regarding you're last post: * GamePlayer/xembed/blenderplayer-wrapper.c --- this file is not maintained or compiled into releases (we should really remove it). * GamePlayer/ghost/GPG_ghost.cpp: pathname = "*home*user*"; --- this is only an example string for printing help text (* are stupid, replaced with /). * Ketsji/KX_PythonInit.cpp: * "/home/me/foo.blend" -> "/home/me/scripts" --- this is a comment. %%%

Cosme Domínguez Díaz commented

2011-10-26 11:23:16 +02:00

%%%Well, you win. Three vs one is too for me so I will not speak again about the temp dir... xD

I know that some people don't like to change the config dirs. Mainly because it's annoying go to a "more" complex route to do the same as before.

But keep in mind that more and more applications are following this standard (see links of first post) so that, in a not too distant future, will really annoying is having to find the config files in two directories /home/user/.config and /home/user/ because a few apps still store their settings in ~/ instead of ~/.config.

So what I think should be done is to adapt that standard as soon as possible in order to the change be less painful.

Campbell, I uploaded a patch that does what you say in 1). I hope it helps.

Thanks all for your comments. I learned a lot with all of you... :-D%%%

%%%Well, you win. Three vs one is too for me so I will not speak again about the temp dir... xD I know that some people don't like to change the config dirs. Mainly because it's annoying go to a "more" complex route to do the same as before. But keep in mind that more and more applications are following this standard (see links of first post) so that, in a not too distant future, will really annoying is having to find the config files in two directories /home/user/.config and /home/user/ because a few apps still store their settings in ~/ instead of ~/.config. So what I think should be done is to adapt that standard as soon as possible in order to the change be less painful. Campbell, I uploaded a patch that does what you say in 1). I hope it helps. Thanks all for your comments. I learned a lot with all of you... :-D%%%

Campbell Barton commented

2011-11-03 10:15:37 +01:00

%%%patch applied, closing.%%%

Campbell Barton commented

2011-11-03 10:15:37 +01:00

Changed status from 'Open' to: 'Resolved'

Campbell Barton closed this issue

2011-11-03 10:15:37 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Add support for XDG Base Directory Specification #28943