webmin / usermin

Usermin source code
http://www.usermin.com/
Other
117 stars 48 forks source link

Date field not decoded in Email viewer. #111

Closed chris001 closed 3 months ago

chris001 commented 3 months ago

usermin email date text format issue 1

usermin email date text format issue 2

jcameron commented 3 months ago

Can you show me the raw Date: header in this email?

iliajie commented 3 months ago

Is this latest Webmin?

chris001 commented 3 months ago

Yes, latest versions: Webmin 2.105.
Usermin 2.005.

usermin email date text format issue 3

jcameron commented 3 months ago

Looks like there's some double-escaping going on here with the date. Does this still happen if you switch to the older Framed Theme in Usermin?

chris001 commented 3 months ago

Yes usermin email date text format issue 4

iliajie commented 3 months ago

We should just allow  , perhaps!

jcameron commented 3 months ago

@iliajie the problem code is here : https://github.com/webmin/webmin/blob/master/web-lib-funcs.pl#L2263

I see no reason for this though, and it makes assumptions about what escaping the caller will do! I think we should just remove that conversion to nbsp.

iliajie commented 3 months ago

It wasn't added by accident. It may break in modules like Webmin Actions Log.

jcameron commented 3 months ago

How would it break anything? Other than perhaps effecting the wrapping ...

iliajie commented 3 months ago

Well, this code is there by a reason. There are modules in Webmin that do splitting in a way that expect time value to be (\S+) in a table cell, and I wasn't sure how many more modules were there doing the same, so I worked around it.

So, for example, if I locally remove this code, this is how things break:

image

It breaks in much worse way in Authentic.

jcameron commented 3 months ago

Oh I see, that's really a bug in the Webmin Actions Log module - it shouldn't be assuming anything about the date format!

I have fixed that here : https://github.com/webmin/webmin/commit/3a275d1d5b696f924be1f11f1b45c6acb4308472

Are there any other modules that split up the date like this?

iliajie commented 3 months ago

Are there any other modules that split up the date like this?

This is the reason why I had to implement the work-around in the first place, as I simply don't know if there are any other modules doing the same or not.

jcameron commented 3 months ago

I looked through the code, and fixed one more case here : https://github.com/webmin/webmin/commit/83c93d8bdca38cd72034f014eff83fa6ef1fccb1

Basically, callers shouldn't assume that make_date has a particular format, and calling split on it is a bug. We should fix all those bugs, instead of trying to solve it inside the make_date function.

iliajie commented 3 months ago

We should fix all those bugs, instead of trying to solve it inside the make_date function.

Yes, agreed! It is always best ideally speaking! I will apply your code and keep looking around in case I find something that's broken. I will also remove mentioned earlier code from make_date sub.

Thanks!

iliajie commented 3 months ago

I have removed that line with a work-around.

jcameron commented 3 months ago

Thanks Ilia!

iliajie commented 3 months ago

Welcome!