Hi,
this patch adds some additional checks to the logic or replacing space with a different character. The idea is to not replace the space if we might run into trouble with the modified name and log an error.
As mentioned in the commit message if the original name already contains the replacement character the original name is used and if the input for the revers operation contains both space and the replacement character the input is returned unmodified as well. As an alternative it would be possible to return NULL and cause an error during the lookup. I think returning the unmodified name is more user friendly. The admin might wonder why the space is not replaced but at least the name is resolved.
With this patch a name like 'abc def_ghi' is return unmodified if the replacement character is '_' . But without the check 'abc_def_ghi' is returned which cannot be resolved back which might cause more trouble than the space in the name.
bye, Sumit
On Mon, Aug 25, 2014 at 06:27:01PM +0200, Sumit Bose wrote:
Hi,
this patch adds some additional checks to the logic or replacing space with a different character. The idea is to not replace the space if we might run into trouble with the modified name and log an error.
As mentioned in the commit message if the original name already contains the replacement character the original name is used and if the input for the revers operation contains both space and the replacement character the input is returned unmodified as well. As an alternative it would be possible to return NULL and cause an error during the lookup. I think returning the unmodified name is more user friendly. The admin might wonder why the space is not replaced but at least the name is resolved.
With this patch a name like 'abc def_ghi' is return unmodified if the replacement character is '_' . But without the check 'abc_def_ghi' is returned which cannot be resolved back which might cause more trouble than the space in the name.
bye, Sumit
Did you consider shouting to syslog so that the failure is better discoverable for the admin?
On Tue, Aug 26, 2014 at 03:56:50PM +0200, Jakub Hrozek wrote:
On Mon, Aug 25, 2014 at 06:27:01PM +0200, Sumit Bose wrote:
Hi,
this patch adds some additional checks to the logic or replacing space with a different character. The idea is to not replace the space if we might run into trouble with the modified name and log an error.
As mentioned in the commit message if the original name already contains the replacement character the original name is used and if the input for the revers operation contains both space and the replacement character the input is returned unmodified as well. As an alternative it would be possible to return NULL and cause an error during the lookup. I think returning the unmodified name is more user friendly. The admin might wonder why the space is not replaced but at least the name is resolved.
With this patch a name like 'abc def_ghi' is return unmodified if the replacement character is '_' . But without the check 'abc_def_ghi' is returned which cannot be resolved back which might cause more trouble than the space in the name.
bye, Sumit
Did you consider shouting to syslog so that the failure is better discoverable for the admin?
New version calling sss_log() as well is attached. Please note that sss_log() might be broken in master, see my other patch for this.
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Tue, Aug 26, 2014 at 05:46:33PM +0200, Sumit Bose wrote:
On Tue, Aug 26, 2014 at 03:56:50PM +0200, Jakub Hrozek wrote:
On Mon, Aug 25, 2014 at 06:27:01PM +0200, Sumit Bose wrote:
Hi,
this patch adds some additional checks to the logic or replacing space with a different character. The idea is to not replace the space if we might run into trouble with the modified name and log an error.
As mentioned in the commit message if the original name already contains the replacement character the original name is used and if the input for the revers operation contains both space and the replacement character the input is returned unmodified as well. As an alternative it would be possible to return NULL and cause an error during the lookup. I think returning the unmodified name is more user friendly. The admin might wonder why the space is not replaced but at least the name is resolved.
With this patch a name like 'abc def_ghi' is return unmodified if the replacement character is '_' . But without the check 'abc_def_ghi' is returned which cannot be resolved back which might cause more trouble than the space in the name.
bye, Sumit
Did you consider shouting to syslog so that the failure is better discoverable for the admin?
New version calling sss_log() as well is attached. Please note that sss_log() might be broken in master, see my other patch for this.
bye, Sumit
Thank you, ACK.
On Mon, Sep 01, 2014 at 04:47:36PM +0200, Jakub Hrozek wrote:
On Tue, Aug 26, 2014 at 05:46:33PM +0200, Sumit Bose wrote:
On Tue, Aug 26, 2014 at 03:56:50PM +0200, Jakub Hrozek wrote:
On Mon, Aug 25, 2014 at 06:27:01PM +0200, Sumit Bose wrote:
Hi,
this patch adds some additional checks to the logic or replacing space with a different character. The idea is to not replace the space if we might run into trouble with the modified name and log an error.
As mentioned in the commit message if the original name already contains the replacement character the original name is used and if the input for the revers operation contains both space and the replacement character the input is returned unmodified as well. As an alternative it would be possible to return NULL and cause an error during the lookup. I think returning the unmodified name is more user friendly. The admin might wonder why the space is not replaced but at least the name is resolved.
With this patch a name like 'abc def_ghi' is return unmodified if the replacement character is '_' . But without the check 'abc_def_ghi' is returned which cannot be resolved back which might cause more trouble than the space in the name.
bye, Sumit
Did you consider shouting to syslog so that the failure is better discoverable for the admin?
New version calling sss_log() as well is attached. Please note that sss_log() might be broken in master, see my other patch for this.
bye, Sumit
Thank you, ACK.
* master: 92d19f76449817dfb125da9510d478a30eed37bc * sssd-1-11: bc4b852c36ee6d36e12592c37e4e3bfe497f8c68
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Sumit Bose