| Age | Commit message (Collapse) | Author |
|---|
|
|
|
The RFC says that the message separator line consists of
> a timestamp indicating the UTC date and time when the message
> was originally received, conformant with the syntax of the
> traditional UNIX 'ctime' output sans timezone (note that the
> use of UTC precludes the need for a timezone indicator);
It also references http://qmail.org/man/man5/mbox.html as an
authoritative source, which says the date "always contains exactly 24
characters in asctime format".
Add this date format for compatibility with tools using the standard
mbox format, in particular git format-patch.
|
|
The readmessage loop clears errstr at start and expects it not to
change unless there is a read error. However, strtotm may use
multiple calls to tmparse while trying to determine the date format,
which may leave errstr non-empty on success. Clear it after chkunix
so that this doesn't get treated as a message read error.
This also fixes handling of naked From lines, which were previously
just warned about, but since the conversion to tmparse they
accidentally triggered a message read failure.
|
|
We have to ensure the size we compute for memmove(...) in `Expunge` is
properly bounded. For certain combinations of inputs we compute an
illegal size causing a suicide:
upas/fs: imap: fetchrsp: fetchrsp: bad idx 7
upas/fs: user: igor; note: sys: trap: fault read addr=0x0 pc=0x214f92
fs 531: suicide: sys: trap: fault read addr=0x0 pc=0x214f92
Stack trace including state of data-structures:
term% acid -l /sys/src/cmd/upas/fs/imap.acid 531
/proc/531/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: lstk()
memmove(p2=0x8e5928,n=0xffffffffffffffe8)+0x42 /sys/src/libc/amd64/memmove.s:36
imap4resp0(imap=0x423c80,mb=0x0,m=0x0)+0x448 /sys/src/cmd/upas/fs/imap.c:522
unexp=0x58b00000000
p=0x425d17
ep=0x425d17
line=0x425d0b
n=0x425d100000058c
verb=0x425d10
op=0x2e6d6f63
imap4resp()+0x1b /sys/src/cmd/upas/fs/imap.c:556
imap4read(imap=0x423c80,mb=0x4239e0)+0x30 /sys/src/cmd/upas/fs/imap.c:928
s=0x425d29
f=0x425d29
n=0x58b0000058c
ll=0x5008a34b0
i=0x425d290000058b
m=0x171bd1f85150e2a6
imap4sync(mb=0x4239e0)+0x62 /sys/src/cmd/upas/fs/imap.c:1059
imap=0x423c80
err=0x0
syncmbox(mb=0x4239e0,doplumb=0x70616d6900000001)+0x5c /sys/src/cmd/upas/fs/mbox.c:76
a=0x58f
n=0x400bc800000000
d=0x0
y=0x0
next=0x171bd206499a9366
m=0x66939a4906d21b17
reader()+0xde /sys/src/cmd/upas/fs/fs.c:1488
t=0x204ca163404153
mb=0x692e6d68656f622f
io()+0x212 /sys/src/cmd/upas/fs/fs.c:1416
n=0x0
main(argc=0x0,argv=0x7fffffffef58)+0x31e /sys/src/cmd/upas/fs/fs.c:353
mboxfile=0x7fffffffef7a
nodflt=0x300000000
srvpost=0x0
v=0x7fffffffef38
_argc=0x6d
_args=0x40d7bd
p=0x400000003
maildir=0x0
mbox=0x0
srvfile=0x0
_main+0x40 /sys/src/libc/amd64/main9.s:15
acid: Imap(0x423c80)
mbox 0x0000000000428b90
freep 0x0000000000423c20
host 0x0000000000423c27
user 0x0000000000423c36
refreshtime 60
cap 0x00
flags 0x05
tag 872
validity 605040277
newvalidity 605040277
nmsg 1419
size 0
f 0x00000000008dd408
nuid 1420
muid 1420
Biobuf bin {
Biobufhdr {
icount -28
ocount 0
rdline 16
runesize 0
state 1
fid 10
flag 0
offset 280380
bsize 8192
bbuf 0x0000000000423d35
ebuf 0x0000000000425d35
gbuf 0x0000000000425cd1
errorf 0x0000000000000000
iof 0x0000000000228d17
aux 0x0000000000000000
}
b end+0x388
}
Biobuf bout {
Biobufhdr {
icount 0
ocount -8192
rdline 0
runesize 0
state 2
fid 10
flag 0
offset 33362
bsize 8192
bbuf 0x0000000000425d9d
ebuf 0x0000000000427d9d
gbuf 0x0000000000427d9d
errorf 0x0000000000000000
iof 0x0000000000228d3a
aux 0x0000000000000000
}
b 0x425d98
}
binit 1
fd 10
The root cause is an integer underflow where we subtract -1 from 0
using an unsigned type.
The above acid trace shows the following values for key local variables:
• nmsg ... 1419
• muid ... 1420
• n ... 1420
• idx ... 1419
The key section of code is /sys/src/cmd/upas/fs/imap.c:516,522
case Expunge:
if(n < 1 || n > imap->muid){
snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
return error;
}
idx = n - 1;
memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));
Plugging the above values into the call to memmove(...) demonstrates the
issue:
memmove(&imap->f[1419], &imap->f[1419 + 1], (1419 - 1419 - 1)*sizeof(imap->f[0]));
^^^^^^^^^^^^^^^
The third argument of memmove(...) is an unsigned size type causing
the size to be a value that is way too large (the stack trace shows
the size to be the unsigned value 0xffffffffffffffe8).
The `Expunge` case can be fixed by amending the bounds checking
condition before the memmove(...):
case Expunge:
if(n < 1 || n > imap->muid || (n - 1) >= imap->nmsg){
^^^^^^^^^^^^^^^^^^^^^
snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
return error;
}
idx = n - 1;
memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));
To add some additional context, this issue has been introduced in
revision:
term% git/export 84c4c81ceecfa8f51949787fc2dbe7b14164a353
Date: Mon, 02 Dec 2019 01:12:19 +0000
Subject: [PATCH] upas/fs imap fixes and improvements
do incremental imap fetches after startup, fixes validity handling,
record flags correctly when we aren't in the process of directly
updating a message, fixes off by one in flag parsing, fixes
mis-indexing messages in sync when we get an unsolicited fetch
response.
...
|
|
Noticed while doing some debugging.
|
|
changeset: 8411:19f6a88ea241
branch: mbp-2011
user: Romano <unobe@cpan.org>
date: Sat Apr 17 14:35:21 2021 -0700
files: sys/src/cmd/upas/fs/imap.c
description:
When an imap fetch fails, it's helpful at times to know the underlying
cause. This provides more details by providing the underlying error
message.
|
|
With ntlm auth, we were trying to set 0 bytes of
the auth struct to its size. The args were clearly
swapped. Fix it.
While we're here, remove some dead code.
|
|
Fixes 3 issues in our upas mkfiles:
- mk/mkfile and send/mkfile were rebuilding
only the rfc822.tab.$O, even though the
header also needed to be rebuilt.
- CLEANFILES had a pattern that would not
get expanded.
- Third, ../upas/mkfile was being included
in the wrong place and making the wrong
rule default.
|
|
Changeset 50ad211fb12f broke the libcommon rule in
mkupas. Deleting the 'mk clean' in the recipe fixes
this.
Cleanup includes deleting UPDATE vars from all mkfiles,
reorganization of vars in TARG,LIB,OFILE,HFILE order,
and deletion of extra vars used for UPDATE.
|
|
|
|
the new date format introduced by the previous commit;
using numeric timezone offsets; needs one character more,
so increase the date format buffer to 31 characters.
|
|
Complete the conversion of upas to remove ctime,
use the new date library, and print time zones
in +hhmm format, instead of NNN format.
This may affect code that expects specific names
for timezones. Fix that code.
|
|
Right now, upasfs exposes header lines as is, without stripping
out new lines. It also documents that it provides one header per
line in the info file.
As a result, when we get a mail with headers that span lines,
our tools get confused.
These split lines are not semantically meaningful. From RFC5322:
2.2.3. Long Header Fields
Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a
multiple-line representation; this is called "folding". The general
rule is that wherever this specification allows for folding white
space (not simply WSP characters), a CRLF may be inserted before any
WSP.
As a result, to simplify processing, we should just strip out the
line separators when exposing the headers from upasfs.
|
|
do not try to parse the m->unixfrom field, it only contains
the unix mail address.
instead, have parseunix() save a pointer into the unixheader
after the unix mail address for the unixdate, and later use
it to derive the mails timestamp.
|
|
|
|
There was a lot of code in upas/fs to deal with dates.
Now there isn't.
|
|
version(5) says:
If the server does not understand the client's version
string, it should respond with an Rversion message (not
Rerror) with the version string the 7 characters
``unknown''.
Pre-lib9p file servers -- all except cwfs(4) -- do return Rerror.
lib9p(2) follows the above spec, although ignoring the next part
concerning comparison after period-stripping. It assumes an
Fcall.version starting with "9P" is correctly formed and returns
the only supported version of the protocol, which seems alright.
This patch brings pre-lib9p servers in accordance with the spec.
|
|
The current logging prints a debug line for every
message in an inbox, which is unusably verbose.
This removes the prints for unchanged messages,
and adds a print for flag changes.
|
|
For big mailboxes with imap4d, ignoring the index and trying to scan
the mailbox concurrently is not very productive. Just wait for the
other upas/fs to write the whole index.
The issue is that imap might time out and make another connection
spawning even more upas/fs instances that all then try to rebuild
the index concurrently.
|
|
|
|
the date, from and replyto fields where unstable, in that the value
read depended on the state of the cache.
fixing the from and replyto fields is easy, we just handle the
substitution in parsebody().
the date field however requires us to put the date822 into the index
so it can be recovered without requiering to reparse the header
(and body, as we might have a message/rfc822 message with promoted
fields).
with these changes, the fields will be consistent and independnet
of the cache state.
a small optimization also has been added:
after parsing the body, attachments and substitution of from/replyto,
the boundary and unixfrom strings are not needed anymore and can
be freed early.
|
|
|
|
parsing the unixheader in mdir fetch routine is the wrong place,
as no invalid character handling has been performed yet. also
the string is not neccesarily null terminated.
avoid duplication with plan9 mbox parsing and just do it in
parseheaders(), which already handles faking the unix headers
for pop3 and imap.
|
|
|
|
make sure we look for the end of the header within the
pointer range, and not accidentally read beyond hend.
also, messages are not null terminated, so this could
even go beyond the email data buffer.
get rid of mimeflag which was only used for some assert
checks.
take header length into account when comparing header
against ignored header strings.
|
|
The mount() and bind() syscalls return -1 on error,
and the mountid sequence number on success.
The manpage states that the mountid sequence number
is a positive integer, but the kernels implementation
currently uses a unsigned 32-bit integer and does not
guarantee that the mountid will not become negative.
Most code just cares about the error, so test for
the -1 error value only.
|
|
Broken by changeset 2cc069392228.
|
|
Currently upas/fs plumbs modify messages only if the flag
changes are made by another imap connection. If the flag
changes are made within the running upas/fs no modify message
is plumbed.
This changes upas/fs to set the modify flag if we made the
change ourself. It also moves the flag setting before the
imap read, so that we don't clobber flag changes coming
from the imap server with our own flags.
(Thanks Tobias Heinicke)
|
|
|
|
|
|
This patch makes 3 changes:
- It makes upas/fs send plumb messages when a message
changes in the background (eg, someone on another imap
connection opens a message and sets the read flag)
- It makes faces not complain when it gets one of these
new modify messages.
- It makes acme/Mail update the flags in the display
when it gets one of these messages.
|
|
|
|
we've only got a few flags, a linear search is good enough,
and is obviously correct; the old search wasn't.
|
|
|
|
do incremental imap fetches after startup, fixes validity handling,
record flags correctly when we aren't in the process of directly
updating a message, fixes off by one in flag parsing, fixes
mis-indexing messages in sync when we get an unsolicited fetch
response.
|
|
|
|
|
|
we were getting nils in the list when there were many references.
this fixes and simplifies the copying loop and makes the code rhyme.
|
|
Since numeric timezone offsets are relative to GMT, initialise zone to
GMT so tm2sec(2) does not assume local time.
Note that if strtotm encounters a timezone *string* and consequently
overwrites zone then we will end up in the same mess since tm2sec(2)
only deals with GMT or local time.
|
|
|
|
with unix.
|
|
move digest pointer into Mtree structrue and embed it into Idx struct
(which is embedded in Message) to avoid one level of indirection
during mtreecmp().
get rid of mtreeisdup(). instead we have mtreeadd() return the old
message in case of a collision. this avoids double lookup.
increase the hash table size for henter() and make it a prime.
|
|
The alarm note is not handled by upas/fs, so if and when it did fire,
the pop3 client process would terminate rendering the entire fs
unresponsive.
|
|
|
|
|
|
|
|
the lru is there to track least recently used messages so
we can evict them from the cache and refetch them again on
demand. for pop3 mailbox, which doesnt provide fetch routine,
the messages should never be put on the freelist.
|
|
|
|
The previous attempt to fix this problem (see changesets b32199e0f90a
and 00ae79a6ba50) caused all calls to cachefree to free the cached
message contents in addition to updating the LRU list. This causes
problems for the POP3 driver since it provides no fetch function; once
a message is evicted from the LRU cache, its contents is lost.
This time we fix cachefree to always update the LRU list but only free
the cached message contents if the driver provides a fetch function or
the force flag is set.
|
|
|
