View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001327SOGoSOPEpublic2011-05-31 05:242011-07-05 18:15
Reporteravoegele Assigned Toludovic  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version1.3.7 
Target Version1.3.8Fixed in Version1.3.8 
Summary0001327: stringByDeletingLastPathComponent must not be used with URLs
Description

On OpenBSD with GNUstep-Base 1.22.0, SOGo stores invalid URLs in the
fields c_location, c_quick_location and c_acl_location in the table
sogo_folder_info.

Additional Information

In SOPE/GDLContentStore/GCSFolderManager.m the method
stringByDeletingLastPathComponent is called with an URL. Unfortunately,
the method not only strips the last path component but also replaces
double slashes by a single slash. For example, "mysql://" is replaced by
"mysql:/". The flawed code fragment:

  • (NSException *) _reallyCreateFolderWithName:[...]
    {
    [...]
    baseURL
    = [[folderInfoLocation absoluteString]
    stringByDeletingLastPathComponent];
    [...]
    }

According to Apple's NSString documentation the method
stringByDeletingLastPathComponent may only be used on file paths: "Note
that this method only works with file paths (not, for example, string
representations of URLs)."

There are at least two other places where
stringByDeletingLastPathComponent is used with URLs:

UI/MailerUI/UIxMailFolderActions.m: path = [[srcURL path]
stringByDeletingLastPathComponent];

SoObjects/SOGo/SOGoGCSFolder.m: [currentURL
stringByDeletingLastPathComponent],

The method is also called in OpenChange/SOGoMAPIFSFolder.m, but only on
the path component of an URL.

Here's an example program that demonstrates the problem:

#import <Foundation/Foundation.h>
int
main(void)
{
NSAutoreleasePool pool;
pool = [NSAutoreleasePool new];
NSURL url = [[NSURL alloc]
initWithString:@"mysql://sogo:secret@localhost:3306/sogo/sogo_sessions_folder"];
NSLog(@"url: %@", [[url absoluteString]
stringByDeletingLastPathComponent]);
return 0;
}

TagsNo tags attached.

Activities

2011-06-01 06:05

 

patch-SOPE_GDLContentStore_GCSFolderManager_m (908 bytes)    
$OpenBSD$

stringByDeletingLastPathComponent must not be used with URLs as it
replaces doubles slashes with a single slash, e.g. "mysql://" with
"mysql:/".

--- SOPE/GDLContentStore/GCSFolderManager.m.orig	Fri May  6 17:57:44 2011
+++ SOPE/GDLContentStore/GCSFolderManager.m	Tue May 31 08:43:12 2011
@@ -748,8 +748,10 @@ static NSCharacterSet *asciiAlphaNumericCS  = nil;
   aclTableName = [tableName stringByAppendingString: @"_acl"];
 
   // TBD: fix SQL injection issues
-  baseURL
-    = [[folderInfoLocation absoluteString] stringByDeletingLastPathComponent];
+  baseURL = [folderInfoLocation absoluteString];
+  NSRange range = [baseURL rangeOfString: @"/" options: NSBackwardsSearch];
+  if (range.location != NSNotFound)
+    baseURL = [baseURL substringToIndex: range.location];
   
   sql = [NSString stringWithFormat: @"INSERT INTO %@"
 		  @"        (c_path, c_path1, c_path2, c_path3, c_path4,"
patch-SOPE_GDLContentStore_GCSFolderManager_m (908 bytes)   
avoegele

avoegele

2011-06-01 06:10

reporter   ~0002514

I've just run the test program on Ubuntu. Under GNUstep 1.20 the double slashes aren't stripped from "mysql://". GNUstep 1.22 strips the double slashes from the URL.
buzzdee

buzzdee

2011-06-05 17:39

reporter   ~0002536

I can verify the problem with postgresql:// database.

I think this change to NSString.m (from the GNUstep Base changelog) broke SOGo:
2011-02-21 Richard Frith-Macdonald <rfm@gnu.org>

    * Source/NSString.m: Modified stringByStandardizingPath and
    stringByDeletingLastPathComponent to better match OSX behavior.

Unfortunately, an equivalent -[NSURL URLByDeletingLastPathComponent] is only for MAC OS X 10.6, and not (yet) implemented in GNUstep base.

However, for the time being, instead of doing this range based approach that Andreas proposed, the URL could also be splitting folderInfoLocation into the base URL, using -[NSURL baseURL] and the path, using -path, and then deleting the last path component from the path, and joining the strings again.

i.e.
baseURL = [NSString stringWithFormat: "%@%@",
[[folderInfoLocation baseURL] absoluteString],
[[folderInfoLocation path] stringByDeletingLastPathComponent]];

Any thoughts on the fix from Andreas or from me? Which one would be the preferred way to go? Let me know and I'd look into it. The problem will probably more pressing, when more distributions will update the gnustep-core...

Sebastian
buzzdee

buzzdee

2011-06-06 14:16

reporter   ~0002538

The idea I had did not worked out, but the patch from Andreas works for me too.
buzzdee

buzzdee

2011-06-17 09:03

reporter   ~0002598

Hi,

gnustep-base-1.22.1 is planned to be released soon. There is a patch in this bug:
https://savannah.gnu.org/patch/index.php?7557
adding URLByDeletingLastPathComponent

I already stated, that it would great if it could be added to this upcoming stable release, since it would make it much easier and the code much cleaner here to use this Method from NSURL.

Sebastian
wsourdeau

wsourdeau

2011-06-18 03:49

viewer   ~0002604

Dev: we could add the methods in NSURL+Utilities and put #if macros depending on the version of GNUstep. That would be a clean solution.
ludovic

ludovic

2011-07-05 18:15

administrator   ~0002659

Fixed: http://mtn.inverse.ca/revision/diff/d68444a3c794ab5baa8363ca087ec5caad13ba7b/with/0b515b8331401536a0de4cc2ca53fce6dd6ca6ed

Thanks for the patch!

Issue History

Date Modified Username Field Change
2011-05-31 05:24 avoegele New Issue
2011-06-01 06:05 avoegele File Added: patch-SOPE_GDLContentStore_GCSFolderManager_m
2011-06-01 06:10 avoegele Note Added: 0002514
2011-06-05 17:39 buzzdee Note Added: 0002536
2011-06-06 14:16 buzzdee Note Added: 0002538
2011-06-15 20:20 ludovic Status new => assigned
2011-06-15 20:20 ludovic Assigned To => ludovic
2011-06-15 20:20 ludovic Target Version => 1.3.8
2011-06-17 09:03 buzzdee Note Added: 0002598
2011-06-18 03:49 wsourdeau Note Added: 0002604
2011-07-05 18:15 ludovic Note Added: 0002659
2011-07-05 18:15 ludovic Status assigned => resolved
2011-07-05 18:15 ludovic Fixed in Version => 1.3.8
2011-07-05 18:15 ludovic Resolution open => fixed