View Issue Details

IDProjectCategoryView StatusLast Update
0002096SOGoWeb Mailpublic2012-11-15 11:49
Reportercnaumer Assigned To 
PrioritynormalSeverityminorReproducibilityrandom
Status closedResolutionfixed 
Product Version2.0.2 
Target Version2.0.2aFixed in Version2.0.2a 
Summary0002096: Corruppted JPG, PDF and DOC
Description

Sending an jpg as attachment can result in the jgp getting corrupted.

TagsNo tags attached.

Activities

2012-11-09 08:34

 

email.txt (2,772,725 bytes)
cnaumer

cnaumer

2012-11-09 08:36

reporter   ~0004811

The attached email ist displayed correctly in TB with attachments. The same views in the webinterface looks like the attached jpg-File

2012-11-09 08:36

 

cnaumer

cnaumer

2012-11-09 08:38

reporter   ~0004812

The Files that are corrupted are 3 bytes larger in this case.

2012-11-09 08:40

 

lemeurt

lemeurt

2012-11-09 10:39

reporter   ~0004814

I'm not sure if this is relevant for this bug I had a corrupted ZIP-attachements issue as well.
The attached ZIP files are correctly openned by thunderbird under windows, but not after downloading them from SOGo Web interface under windows.

I've found out that the ZIP file, when downloaded from the SOGo Web interface could only be openned with 7Zip but not from the windows built-in ZIP archiver anymore.

wsourdeau

wsourdeau

2012-11-09 10:47

viewer   ~0004815

There is a potential fix in sope-mime which was commited a few days ago but which is already availbable in the nightly builds.

Could you try with those packages?

lemeurt

lemeurt

2012-11-09 11:50

reporter   ~0004817

I've compiled both SOPE and SOGo from the latest Git repository as instructed on the FAQ.
I still ahev the issue with the ZIP files.

lemeurt

lemeurt

2012-11-09 11:57

reporter   ~0004818

I've tested the provided JPG with the GIT master branch freshly compiled and get the same result as cnaumer

ludovic

ludovic

2012-11-12 12:01

administrator   ~0004822

Files are not corrupted when being sent - it's the reading/viewing portion of SOGo that is broken.

flevee

flevee

2012-11-12 13:09

reporter   ~0004824

I downgraded our 2.0.2-1 SOGo server to the 2.0.0-1 version (Debian packages)
and there is no issue with the enclosed files anymore.
Please note that I used a snapshot of the server so the companion libraries of Sogo are the ones which were released with the 2.0.0-1 version.

ludovic

ludovic

2012-11-12 14:44

administrator   ~0004827

Can you apply this patch to SOPE?

It removes a lot of premature optimizations that cause issues.

2012-11-12 14:44

 

remove-premature-optimizations.diff (22,299 bytes)   
diff --git a/sope-core/NGStreams/NGByteBuffer.m b/sope-core/NGStreams/NGByteBuffer.m
index 014b857..842cdd0 100644
--- a/sope-core/NGStreams/NGByteBuffer.m
+++ b/sope-core/NGStreams/NGByteBuffer.m
@@ -19,12 +19,16 @@
   02111-1307, USA.
 */
 
-#include "NGStreamProtocols.h"
 #include "NGByteBuffer.h"
 #include "common.h"
-
 #include <sys/time.h>
 
+typedef struct NGByteBufferLA {
+  unsigned char byte;
+  char          isEOF:1;
+  char          isFetched:1;
+} LA_NGByteBuffer;
+
 @implementation NGByteBuffer
 
 static BOOL  ProfileByteBuffer = NO;
@@ -40,13 +44,17 @@ static Class DataStreamClass = Nil;
   DataStreamClass   = NSClassFromString(@"NGDataStream");
 }
 
++ (int)version {
+  return [super version] + 1;
+}
+
 + (id)byteBufferWithSource:(id<NGStream>)_source la:(unsigned)_la {
   if (_source            == nil)            return nil;
   if (*(Class *)_source == DataStreamClass) return _source;
   return [[[self alloc] initWithSource:_source la:_la] autorelease];
 }
 
-- (id)initWithSource:(NSObject <NGStream> *)_source la:(unsigned)_la {
+- (id)initWithSource:(id<NGStream>)_source la:(unsigned)_la {
   if (_source == nil) {
     [self release];
     return nil;
@@ -66,21 +74,21 @@ static Class DataStreamClass = Nil;
     // Find first power of 2 >= to requested size
     for (size = 2; size < _la; size *=2);
     
-    self->laImpl = (int (*)(id, SEL, unsigned))
-      [self methodForSelector: @selector (la:)];
-    self->sourceReadByte = (int(*)(id, SEL))
-      [_source methodForSelector: @selector (readByte)];
-    self->sourceReadBytes = (int (*)(id, SEL, void *, unsigned))
-      [_source methodForSelector: @selector (readBytes:count:)];
-
-    self->la = malloc(size * sizeof(unsigned char));
+    self->la = malloc(sizeof(LA_NGByteBuffer) * size + 4);
+    memset(self->la, 0, sizeof(LA_NGByteBuffer) * size);
 
     self->bufLen      = size;
     self->sizeLessOne = self->bufLen - 1;
     self->headIdx     = 0;
-    self->freeIdx = 0;
-    self->EOFIdx = 0;
     self->wasEOF      = NO;
+    if ([self->source respondsToSelector:@selector(methodForSelector:)]) {
+      self->readByte = (int(*)(id, SEL))
+        [(NSObject *)self->source methodForSelector:@selector(readByte)];
+    }
+    if ([self respondsToSelector:@selector(methodForSelector:)]) {
+      self->laFunction = (int(*)(id, SEL, unsigned))
+        [(NSObject *)self methodForSelector:@selector(la:)];
+    }
   }
   return self;
 }
@@ -116,210 +124,206 @@ static Class DataStreamClass = Nil;
 
 /* operations */
 
-/* this function reads bytes from self->la *without* increasing self->headIdx */
-static size_t readAllFromBuffer(NGByteBuffer *self,
-                                unsigned char *dest, size_t len) {
-  size_t required, lastBytesCount;
-  unsigned localHeadIdx, localNextHeadIdx;
-
-  required = self->freeIdx - self->headIdx;
-  if (required > 0) {
-    if (required > len) {
-      required = len;
-    }
-
-    localHeadIdx = self->headIdx & self->sizeLessOne;
-    localNextHeadIdx = (self->headIdx + required) & self->sizeLessOne;
-    if (localHeadIdx < localNextHeadIdx) {
-      memcpy(dest, self->la + localHeadIdx, required);
-    }
-    else {
-      lastBytesCount = self->bufLen - localHeadIdx;
-      memcpy(dest, self->la + localHeadIdx, lastBytesCount);
-      memcpy(dest + lastBytesCount, self->la, required - lastBytesCount);
-    }
-  }
-
-  return required;
-}
-
-/* this function reads *all* bytes from source, unless an exception was
-   returned. In all case it does *not* increase self->headIdx nor
-   self->freeIdx. */
-static size_t readAllFromSource(NGByteBuffer *self,
-                                unsigned char *dest, size_t len) {
-  register size_t totalReadCnt = 0;
-  register int readCnt;
-
-  while (totalReadCnt < len) {
-    readCnt = self->sourceReadBytes(self->source,
-                                    @selector (readBytes:count:),
-                                    dest + totalReadCnt,
-                                    len - totalReadCnt);
-    if (readCnt == NGStreamError) {
-      NSException *exc = [self->source lastException];
-      if ([exc isKindOfClass:[NGEndOfStreamException class]]) {
-        self->wasEOF = YES;
-      }
-      else {
-        [exc raise];
-      }
-      break;
-    }
-    else {
-      totalReadCnt += readCnt;
-    }
-  }
-
-  return totalReadCnt;
-}
-
 - (int)readByte {
-  int byte = self->laImpl(self, @selector (la:), 0);
+  int byte = (self->laFunction == NULL)
+    ? [self la:0]
+    : self->laFunction(self, @selector(la:), 0);
   [self consume];
   return byte;
 }
 
 - (unsigned)readBytes:(void *)_buf count:(unsigned)_len {
-  size_t totalReadCnt, readCnt;
-
-  if (self->wasEOF && self->headIdx == self->EOFIdx)
-    [NGEndOfStreamException raiseWithStream:self->source];
-
-  totalReadCnt = readAllFromBuffer(self, _buf, _len);
-  self->headIdx += totalReadCnt;
-  if (totalReadCnt < _len) {
-    readCnt = readAllFromSource(self,
-                                _buf + totalReadCnt, _len - totalReadCnt);
-    totalReadCnt += readCnt;
-    /* if we are here, it means that readAllFromBuffer gave headIdx the same
-       value as freeIdx */
-    self->headIdx += readCnt;
-    self->freeIdx = self->headIdx;
-    if (self->wasEOF) {
-      self->EOFIdx = self->headIdx;
-    }
-  }
+  if (_len == 0)
+    return 0;
 
-  return totalReadCnt;
+  if (!(self->la[(self->headIdx & self->sizeLessOne)].isFetched)) {
+    int byte = [self readByte];
 
+    if (byte == -1)
+      [NGEndOfStreamException raiseWithStream:self->source];
+
+    ((char *)_buf)[0] = byte;
+    return 1;
+  }
+  else {
+    unsigned      cnt    = 0;
+    int           idxCnt = self->headIdx & sizeLessOne;
+    unsigned char buffer[self->bufLen];
+    
+    while (self->la[idxCnt].isFetched && cnt < _len && cnt < bufLen) {
+      buffer[cnt] = self->la[idxCnt].byte;
+      cnt++;
+      idxCnt = (cnt + self->headIdx) & sizeLessOne;
+    }
+    memcpy(_buf, buffer, cnt);
+    [self consume:cnt];
+    return cnt;
+  }
+  return 0;
 }
 
 - (int)la:(unsigned)_la {
   // TODO: huge method, should be split up
-  int result;
-  register unsigned idx;
+  volatile unsigned result, idx;
+  unsigned i = 0;
+  
+  result = -1;
+  *(&idx) = (_la + self->headIdx) & self->sizeLessOne;
   
   if (_la > self->sizeLessOne) {
     [NSException raise:NSRangeException
                  format:@"tried to look ahead too far (la=%d, max=%d)", 
                   _la, self->bufLen];
   }
-
-  idx = self->headIdx + _la;
-
-  /* a safeguard against infinite loops in parsers */
-  if (self->lastLaIdx == idx) {
-    self->lastLaIdxCount++;
-    if (self->lastLaIdxCount > 53)
-      [NSException raise: NSInternalInconsistencyException
-                  format: @"invoked %s an unreasonable amount of times (%d)",
-                   __PRETTY_FUNCTION__, self->lastLaIdxCount];
-  }
-  else {
-    self->lastLaIdx = idx;
-    self->lastLaIdxCount = 0;
-  }
-
-  if (idx < self->freeIdx) {
-    result = self->la[idx & self->sizeLessOne];
+  
+  if (self->wasEOF) {
+    result = (!self->la[idx].isFetched || self->la[idx].isEOF)
+      ? -1 : self->la[idx].byte;
+    return result;
   }
-  else if (self->wasEOF) {
-    result = -1;
+  
+  if (self->la[idx].isFetched) {
+    result = (self->la[idx].isEOF) ? -1 : self->la[idx].byte;
+    return result;
   }
-  else {
-    unsigned max, localFreeIdx;
-    register unsigned len, readCnt;
 
+  *(&i) = 0;
+  for (i = 0;
+       i < _la &&
+         self->la[(self->headIdx + i) & self->sizeLessOne].isFetched;
+       i++);
+  
+  /* 
+     If we should read more than 5 bytes, we take the time costs of an
+     exception handler 
+  */
+  if ((_la - i + 1) <= 5) {
+    while (i <= _la) {
 #if DEBUG
-    struct timeval tv;
-    double         ti = 0.0;
+      struct timeval tv;
+      double         ti = 0.0;
 #endif
           
+      int byte = 0;
+
 #if DEBUG
-    if (ProfileByteBuffer) {
-      gettimeofday(&tv, NULL);
-      ti =  (double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0);
-    }
+      if (ProfileByteBuffer) {
+        gettimeofday(&tv, NULL);
+        ti =  (double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0);
+      }
 #endif
+      byte = (self->readByte == NULL)
+        ? [self->source readByte]
+        : (int)self->readByte(self->source, @selector(readByte));
 
-    max = idx - self->freeIdx + 1;
-
-    localFreeIdx = self->freeIdx & self->sizeLessOne;
-    len = self->bufLen - localFreeIdx;
-    if (len > max) {
-      len = max;
-    }
-
-    /* fill last bytes of buffer, from the position pointed at by freeIdx */
-    readCnt = readAllFromSource(self, self->la + localFreeIdx, len);
-    self->freeIdx += readCnt;
-    if (self->wasEOF) {
-      self->EOFIdx = self->freeIdx;
-    }
-    else if (readCnt < max) {
-      /* if needed fill first bytes of buffer */
-      len = max - readCnt;
-      readCnt = readAllFromSource(self, self->la, len);
-      self->freeIdx += readCnt;
-      if (self->wasEOF) {
-        self->EOFIdx = self->freeIdx;
+#if DEBUG
+      if (ProfileByteBuffer) {
+        gettimeofday(&tv, NULL);
+        ti = (double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0) - ti;
+        if (ti > 0.01) {
+          fprintf(stderr, "[%s] <read bytes from stream> : time "
+                  "needed: %4.4fs\n",
+                  __PRETTY_FUNCTION__, ti < 0.0 ? -1.0 : ti);
+        }
+      }
+#endif
+    
+      if (byte == -1) {  // EOF was reached
+        self->wasEOF = YES;
+        break;
       }
+      else {
+        int ix = (self->headIdx + i) & self->sizeLessOne;
+        self->la[ix].byte      = byte;
+        self->la[ix].isFetched = 1;
+      }
+      i++;
     }
-
-#if DEBUG
-    if (ProfileByteBuffer) {
-      gettimeofday(&tv, NULL);
-      ti = (double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0) - ti;
-      if (ti > 0.01) {
-        fprintf(stderr, "[%s] <read bytes from stream> : time "
-                "needed: %4.4fs\n",
-                __PRETTY_FUNCTION__, ti < 0.0 ? -1.0 : ti);
+  }
+  else {
+    BOOL readStream = YES;
+    NSException *exc = nil;
+    
+    while (readStream) {
+      int  cntReadBytes  = 0;
+      int  cnt           = 0;
+      int  desiredBytes  = _la - i+1;
+      char *tmpBuffer;
+
+      // TODO: check whether malloc is used for sufficiently large blocks!
+      tmpBuffer = malloc(desiredBytes + 2);
+
+      cntReadBytes = (self->readBytes == NULL)
+        ? [self->source readBytes:tmpBuffer count:desiredBytes]
+        : self->readBytes(self->source, @selector(readBytes:count:),
+                          tmpBuffer, desiredBytes);
+          
+      if (cntReadBytes == NGStreamError) {
+        exc = [[self->source lastException] retain];
+        break;
       }
+      else {
+        if (cntReadBytes == desiredBytes)
+          readStream = NO;
+
+        cnt = 0;
+        while (cntReadBytes > 0) {
+          int ix = (self->headIdx + i) & self->sizeLessOne;
+          self->la[ix].byte      = tmpBuffer[cnt];
+          self->la[ix].isFetched = 1;
+          i++;
+          cnt++;
+          cntReadBytes--;
+        }
+      }
+          
+      if (tmpBuffer) free(tmpBuffer);
     }
-#endif
-
-    if (idx < self->freeIdx) {
-      result = self->la[idx & self->sizeLessOne];
+    if (exc) {
+      if (![exc isKindOfClass:[NGEndOfStreamException class]]) {
+        [self setLastException:exc];
+        return NGStreamError;
+      }
+      self->wasEOF = YES;
     }
-    else {
-      result = -1;
+  }
+  
+  if (self->wasEOF) {
+    while (i <= _la) {
+      self->la[(self->headIdx + i) & self->sizeLessOne].isEOF = YES;
+      i++;
     }
   }
-
+  
+  result = (self->la[idx].isEOF) ? -1 : self->la[idx].byte;
   return result;
 }
 
 - (void)consume {
-  if (self->headIdx == self->freeIdx) {
-    self->laImpl(self, @selector (la:), 0);
-  }
-  else if (self->headIdx > self->freeIdx) {
-    [NSException raise: NSRangeException
-                format: @"a buffer inconsistency was detected"];
+  int idx = self->headIdx & sizeLessOne;
+  
+  if (!(self->la[idx].isFetched)) {
+    (self->laFunction == NULL)
+      ? [self la:0]
+      : self->laFunction(self, @selector(la:), 0);
   }
+  self->la[idx].isFetched = 0;
   self->headIdx++;
 }
 
 - (void)consume:(unsigned)_cnt {
-  unsigned nextHead, needed;
-
-  nextHead = self->headIdx + _cnt;
-  if (nextHead >= self->freeIdx) {
-    needed = nextHead - self->freeIdx + 1;
-    self->laImpl(self, @selector (la:), needed);
+  while (_cnt > 0) {
+    int idx = self->headIdx & sizeLessOne;
+    
+    if (!(self->la[idx].isFetched))
+      (self->laFunction == NULL)
+        ? [self la:0]
+        : self->laFunction(self, @selector(la:), 0);
+
+    self->la[idx].isFetched = 0;
+    self->headIdx++;
+    _cnt--;
   }
-  self->headIdx = nextHead;
 }
 
 /* description */
diff --git a/sope-core/NGStreams/NGStreams/NGByteBuffer.h b/sope-core/NGStreams/NGStreams/NGByteBuffer.h
index 7c69c04..b4f6603 100644
--- a/sope-core/NGStreams/NGStreams/NGByteBuffer.h
+++ b/sope-core/NGStreams/NGStreams/NGByteBuffer.h
@@ -43,28 +43,22 @@ struct NGByteBufferLA;
 @interface NGByteBuffer : NGFilterStream
 {
 @protected
-  unsigned char *la;
+  struct NGByteBufferLA *la;
 
   unsigned bufLen;
   BOOL     wasEOF;
   unsigned headIdx;
-  unsigned freeIdx; /* first byte index that has not been fetched */
-  unsigned EOFIdx; /* max byte index ever + 1 */
   unsigned sizeLessOne;
 
-  unsigned lastLaIdx;
-  unsigned lastLaIdxCount;
-
-  int (*laImpl)(id, SEL, unsigned);
-  int (*sourceReadByte)(id, SEL);
-  int (*sourceReadBytes)(id, SEL, void *, unsigned);
+  int (*readByte)(id, SEL);
+  int (*laFunction)(id, SEL, unsigned);
 }
 
 /*
   Initialize a byte buffer with a lookahead depth of _la bytes.
 */
 + (id)byteBufferWithSource:(id<NGStream>)_stream la:(unsigned)_la;
-- (id)initWithSource:(NSObject <NGStream> *)_stream la:(unsigned)_la;
+- (id)initWithSource:(id<NGStream>)_stream la:(unsigned)_la;
 
 // LA
 - (int)la:(unsigned)_lookaheadPosition;
diff --git a/sope-mime/NGImap4/NGImap4ResponseParser.m b/sope-mime/NGImap4/NGImap4ResponseParser.m
index 8d45650..aaeac0a 100644
--- a/sope-mime/NGImap4/NGImap4ResponseParser.m
+++ b/sope-mime/NGImap4/NGImap4ResponseParser.m
@@ -57,16 +57,17 @@
 
 @implementation NGImap4ResponseParser
 
-static __inline__ int _la(NGImap4ResponseParser *self, unsigned _laCnt) {
-  register unsigned char c;
-  register unsigned pos = _laCnt;
-
-  while ((c = self->la(self->buffer, @selector(la:), pos)) == '\r')
-    pos++;
+#define __la(__SELF__, __PEEKPOS) \
+  ((__SELF__->la == NULL) \
+    ? [__SELF__->buffer la:__PEEKPOS]\
+    : __SELF__->la(__SELF__->buffer, @selector(la:), __PEEKPOS))
 
-  return c;
+static __inline__ int _la(NGImap4ResponseParser *self, unsigned _laCnt) {
+  register unsigned char c = __la(self, _laCnt);
+  return (c == '\r')
+    ? _la(self, _laCnt + 1)
+    : c;
 }
-
 static __inline__ BOOL _matchesString(NGImap4ResponseParser *self, 
 				      const char *s)
 {
@@ -166,7 +167,7 @@ static NSNull           *null   = nil;
   
   if (Imap4MMDataBoundary < 10)
     /* Note: this should be larger than a usual header size! */
-    Imap4MMDataBoundary = 1 << 20;
+    Imap4MMDataBoundary = 2 * LaSize;
   
   StrClass  = [NSString class];
   NumClass  = [NSNumber class];
@@ -194,11 +195,13 @@ static NSNull           *null   = nil;
     id s;
     
     s = [(NGBufferedStream *)[NGBufferedStream alloc] initWithSource:_stream];
-    self->buffer = [[NGByteBuffer alloc] initWithSource:s la:LaSize];
+    self->buffer = [NGByteBuffer alloc];
+    self->buffer = [self->buffer initWithSource:s la:LaSize];
     [s release];
     
-    self->la = (int(*)(id, SEL, unsigned))
-      [self->buffer methodForSelector:@selector(la:)];
+    if ([self->buffer respondsToSelector:@selector(methodForSelector:)])
+      self->la = (int(*)(id, SEL, unsigned))
+        [self->buffer methodForSelector:@selector(la:)];
     
     self->debug = debugOn;
   }
@@ -270,7 +273,6 @@ static NSNull           *null   = nil;
     l0 = _la(self, 0);
     
     if (l0 == '*') { /* those starting with '* ' */
-      _consume(self, 1);
       _parseUntaggedResponse(self, result);
       if ([result objectForKey:@"bye"]) {
         endOfCommand = YES;
@@ -334,45 +336,17 @@ static void _parseSieveRespone(NGImap4ResponseParser *self,
     return;
 }
 
-static NSUInteger _removeCRLF(unsigned char *buffer, size_t len) {
-  NSUInteger offset;
-  register size_t new_pos, last_pos = 0;
-  unsigned char *chr_ptr, *src_ptr;
-
-  new_pos = 0;
-  offset = 0;
-  chr_ptr = memchr(buffer, '\r', len);
-  while (chr_ptr) {
-    last_pos = new_pos + 1;
-    new_pos = (chr_ptr - buffer);
-    if (last_pos > 1) {
-      offset++;
-      src_ptr = buffer + last_pos;
-      memmove(src_ptr - offset, src_ptr, (new_pos - last_pos));
-    }
-    chr_ptr = memchr(chr_ptr + 1, '\r', len - new_pos - 1);
-  }
-  if (last_pos > 0) {
-    last_pos = new_pos + 1;
-    if (last_pos < len) {
-      offset++;
-      src_ptr = buffer + last_pos;
-      memmove(src_ptr - offset, src_ptr, len - last_pos);
-    }
-  }
-
-  return offset;
-}
-
 - (NSData *)_parseDataToFile:(unsigned)_size {
   // TODO: move to own method
   // TODO: do not use NGFileStream but just fopen/fwrite
   static NSProcessInfo *Pi = nil;
   NGFileStream  *stream;
   NSData        *result;
-  unsigned char buf[LaSize + 1];
-  unsigned      remaining;
+  unsigned char buf[LaSize + 2];
+  unsigned char tmpBuf[LaSize + 2];
+  unsigned      wasRead = 0;
   NSString      *path;
+  signed char   lastChar; // must be signed
       
   if (debugDataOn) [self logWithFormat:@"  using memory mapped data  ..."];
       
@@ -391,19 +365,44 @@ static NSUInteger _removeCRLF(unsigned char *buffer, size_t len) {
     [self setLastException:[e autorelease]];
     return nil;
   }
+      
+  lastChar = -1;
+  while (wasRead < _size) {
+    unsigned readCnt, bufCnt, tmpSize, cnt, tmpBufCnt;
 
-  remaining = _size;
-  buf[LaSize] = '\0';
-  while (remaining > 0) {
-    unsigned readCnt;
-    NSUInteger offset;
+    bufCnt = 0;
+        
+    if (lastChar != -1) {
+      buf[bufCnt++] = lastChar;
+      lastChar = -1;
+    }
+        
+    [self->buffer la:(_size - wasRead <  LaSize) 
+	 ? (_size - wasRead)
+	 : LaSize];
+        
+    readCnt = [self->buffer readBytes:buf+bufCnt count:_size - wasRead];
+        
+    wasRead+=readCnt;
+    bufCnt +=readCnt;
 
-    readCnt = [self->buffer readBytes:buf count:(remaining < LaSize) ? remaining : LaSize];
-    offset = _removeCRLF(buf, readCnt);
-    remaining -= readCnt;
-    
-    [stream writeBytes:buf count:readCnt-offset];
+    tmpSize   = bufCnt - 1;
+    cnt       = 0;
+    tmpBufCnt = 0;
+        
+    while (cnt < tmpSize) {
+      if ((buf[cnt] == '\r') && (buf[cnt+1] == '\n')) {
+	cnt++;
+      }
+      tmpBuf[tmpBufCnt++] = buf[cnt++];
+    }
+    if (cnt < bufCnt) {
+      lastChar = buf[cnt];
+    }
+    [stream writeBytes:tmpBuf count:tmpBufCnt];
   }
+  if (lastChar != -1)
+    [stream writeBytes:&lastChar count:1];
   
   [stream close];
   [stream release]; stream = nil;
@@ -414,27 +413,43 @@ static NSUInteger _removeCRLF(unsigned char *buffer, size_t len) {
 }
 - (NSData *)_parseDataIntoRAM:(unsigned)_size {
   /* parses data into a RAM buffer (NSData) */
-  register unsigned char *buf;
-  // register unsigned char firstChar, nextChar;
-  NSUInteger wasRead, offset;
-  NSData *result;
-
-  buf = malloc((_size + 1) * sizeof(char));
-
-  wasRead = 0;
+  unsigned char *buf = NULL;
+  unsigned char *tmpBuf;
+  unsigned      wasRead   = 0;
+  unsigned      cnt, tmpBufCnt, tmpSize;
+  NSData        *result;
+          
+  buf = calloc(_size + 10, sizeof(char));
+    
   while (wasRead < _size) {
-    self->la(self->buffer, @selector (la:), (_size - wasRead <  LaSize) ? (_size - wasRead) : LaSize);
+    [self->buffer la:(_size - wasRead <  LaSize) ? (_size - wasRead) : LaSize];
+            
     wasRead += [self->buffer readBytes:(buf + wasRead) count:(_size-wasRead)];
   }
-  *(buf + _size) = '\0';
- 
+  
   /* normalize response  \r\n -> \n */
-
-  offset = _removeCRLF(buf, wasRead);
-  result = [DataClass dataWithBytesNoCopy:buf
-                      length:wasRead-offset
-                      freeWhenDone:YES];
-
+	
+  tmpBuf    = calloc(_size + 10, sizeof(char));
+  cnt       = 0;
+  tmpBufCnt = 0;
+  tmpSize   = _size == 0 ? 0 : _size - 1;
+  while (tmpBufCnt < tmpSize && cnt < _size) {
+    if ((buf[cnt] == '\r') && (buf[cnt + 1] == '\n'))
+      cnt++; /* skip \r */
+      
+    tmpBuf[tmpBufCnt] = buf[cnt];
+    tmpBufCnt++;
+    cnt++;
+  }
+  if (cnt < _size) {
+    tmpBuf[tmpBufCnt] = buf[cnt];
+    tmpBufCnt++;
+    cnt++;
+  }
+    
+  result = [DataClass dataWithBytesNoCopy:tmpBuf length:tmpBufCnt];
+    
+  if (buf != NULL) free(buf); buf = NULL;
   return result;
 }
 - (NSData *)_parseData {
@@ -584,13 +599,10 @@ static void _parseUntaggedResponse(NGImap4ResponseParser *self,
   // TODO: is it really required by IMAP4 that responses are uppercase?
   // TODO: apparently this code *breaks* with lowercase detection on!
   unsigned char l0, l1 = 0;
-
-  l0 = _la(self, 0);
-  if (l0 == ' ') {
-    _consume(self, 1);
-    l0 = _la(self, 0);
-  }
+  _consumeIfMatch(self, '*');
+  _consumeIfMatch(self, ' ');
   
+  l0 = _la(self, 0);
   switch (l0) {
   case 'A':
     if ([self _parseACLResponseIntoHashMap:result_])
@@ -2496,10 +2508,9 @@ static BOOL _parseNoUntaggedResponse(NGImap4ResponseParser *self,
 }
 
 static BOOL _parseOkUntaggedResponse(NGImap4ResponseParser *self,
-                                     NGMutableHashMap *result_)
+                                     NGMutableHashMap *result_) 
 {
-  /* we know the first element is 'O', since the caller has tested it */
-  if (!((_la(self, 1)=='K') && (_la(self, 2)==' ')))
+  if (!((_la(self, 0)=='O') && (_la(self, 1)=='K') && (_la(self, 2)==' ')))
     return NO;
   
   _consume(self, 3);
@@ -2676,11 +2687,9 @@ static __inline__ void _consume(NGImap4ResponseParser *self, unsigned _cnt) {
 
   if (_cnt == 0)
     return;
-
-  if (self->la(self->buffer, @selector(la:), _cnt - 1) == '\r') {
-    _cnt++;
-  }
-
+  
+  _cnt +=  (__la(self, _cnt - 1) == '\r') ? 1 : 0;
+  
   if (self->debug) {
     unsigned cnt;
     
lemeurt

lemeurt

2012-11-13 03:41

reporter   ~0004833

I'll give it a try today and will report back.

lemeurt

lemeurt

2012-11-13 03:53

reporter   ~0004834

Last edited: 2012-11-13 04:17

I can't apply your patch to the master Git branch.
is there a specific Tag I should use istead of using the latest branch ?

root@sogodevel:/home/sogod/sope# patch -p0 --dry-run < patch.txt
patching file b/sope-core/NGStreams/NGByteBuffer.m
Hunk 0000001 FAILED at 19.
Hunk 0000002 FAILED at 40.
Hunk 0000003 FAILED at 66.
Hunk 0000004 FAILED at 116.
4 out of 4 hunks FAILED -- saving rejects to file b/sope-core/NGStreams/NGByteBuffer.m.rej
patching file b/sope-core/NGStreams/NGStreams/NGByteBuffer.h
Hunk 0000001 FAILED at 43.
1 out of 1 hunk FAILED -- saving rejects to file b/sope-core/NGStreams/NGStreams/NGByteBuffer.h.rej
patching file b/sope-mime/NGImap4/NGImap4ResponseParser.m
Hunk 0000001 FAILED at 57.
Hunk 0000002 FAILED at 166.
Hunk 0000003 FAILED at 194.
Hunk 0000004 FAILED at 270.
Hunk 0000005 FAILED at 334.
Hunk 0000006 FAILED at 391.
Hunk 0000007 FAILED at 414.
Hunk 0000008 FAILED at 584.
Hunk 0000009 FAILED at 2496.
Hunk 0000010 FAILED at 2676.
10 out of 10 hunks FAILED -- saving rejects to file b/sope-mime/NGImap4/NGImap4ResponseParser.m.rej

ludovic

ludovic

2012-11-13 07:04

administrator   ~0004835

Get the latest revision of SOPE from git and simply apply the patch with "patch -p1 < the file".

lemeurt

lemeurt

2012-11-13 08:00

reporter   ~0004836

Oups.. now I'm feeling completely stupid by not having been able to modify my '-p' option ;-)

Anyway, you patch fixes the ZIP issue I've reported, as well as the JPG issue reported in this ticket.

I've not been able to test with a pdf file causing an issue because I don't have one.

ludovic

ludovic

2012-11-13 08:37

administrator   ~0004837

Fix pushed:

https://github.com/inverse-inc/sope/commit/a18bb288f890db5747911bf2c74382e59583fde4

Issue History

Date Modified Username Field Change
2012-11-09 08:34 cnaumer New Issue
2012-11-09 08:34 cnaumer File Added: email.txt
2012-11-09 08:36 cnaumer Note Added: 0004811
2012-11-09 08:36 cnaumer File Added: 121108_Trafos_MC2_SkimMilk_2.jpg
2012-11-09 08:38 cnaumer Note Added: 0004812
2012-11-09 08:40 cnaumer File Added: 121108_Trafos_MC2_SkimMilk_2_ok.jpg
2012-11-09 10:39 lemeurt Note Added: 0004814
2012-11-09 10:47 wsourdeau Note Added: 0004815
2012-11-09 11:50 lemeurt Note Added: 0004817
2012-11-09 11:57 lemeurt Note Added: 0004818
2012-11-12 12:01 ludovic Note Added: 0004822
2012-11-12 13:09 flevee Note Added: 0004824
2012-11-12 14:44 ludovic Note Added: 0004827
2012-11-12 14:44 ludovic File Added: remove-premature-optimizations.diff
2012-11-12 15:04 ludovic Target Version => 2.0.3
2012-11-13 03:41 lemeurt Note Added: 0004833
2012-11-13 03:53 lemeurt Note Added: 0004834
2012-11-13 04:17 lemeurt Note Edited: 0004834
2012-11-13 07:04 ludovic Note Added: 0004835
2012-11-13 08:00 lemeurt Note Added: 0004836
2012-11-13 08:37 ludovic Note Added: 0004837
2012-11-13 08:37 ludovic Status new => closed
2012-11-13 08:37 ludovic Resolution open => fixed
2012-11-13 08:37 ludovic Fixed in Version => 2.0.3
2012-11-15 11:49 francis Fixed in Version 2.0.3 => 2.0.2a
2012-11-15 11:49 francis Target Version 2.0.3 => 2.0.2a