Can not change reporting user name

delphi package - automated exception handling

Can not change reporting user name

Postby slemmnord » Wed Jan 25, 2017 10:14 pm

Hi!

Got two other issues to report.

When handling reporting (via PHP post as i mentioned in previous posting), one would expect changing MESettings.MailFrom in standard e-mail format (sender name <address@server.tld>) would make the report pop in my mail box with the name user typed into the report send dialog. However we do not get the name part of reporting user.

This is because in madexcept.pas, line 9264 there is call:
AddHeaderData('From: ' + FormatMailAddr(mailFrom, false));

I suggest change it to:
AddHeaderData('From: ' + FormatMailAddr(mailFrom, true));

Unless there is some good reason not to allow name part? Maybe this makes sense in e-mail based reporting.. i didn't look but for php post i see no obvious reason.


And third issue here (because also related to mailfrom):

I have registered a global handler to override default ME dialog and to take users comments and e-mail.
RegisterExceptionHandler(GlobalMadExceptionHandler, stTrySyncCallAlways, epMainPhase);

In the handler i include to the report the users name together with email. Changing exceptIntf.MailFrom has no effect because SendBugReport creates a new instance of IMESettings and overwrites whatever was set via exceptIntf.MailFrom. So thus only way now is to change MESettings.MailFrom. I think this is unintuitive (i even saw previous questions about this in forum). IMO it would be more logical to allow change exceptIntf.MailFrom, same as all the other report instance related adjustments. The SendBugReportEx is quite complex so this wont be a one-liner to change i realize. Perhaps it could take from exception header first, and if not found only then take default from MESettings?

That's all for now. And thanks for making ME!
slemmnord
 
Posts: 19
Joined: Fri Jan 17, 2014 1:04 am

Re: Can not change reporting user name

Postby madshi » Mon Jan 30, 2017 12:29 pm

I can find that AddHeaderData only for HTTP uploading, is that correct? I'm not sure why I'm stripping the name there, but I think I probably had a reason for that. Does HTTP posting support "From:" email addresses with names? Recently also some user reported that some HTTP servers reject fields with "<" or ">" in it because it could contain some sort of script.

I don't understand your 2nd issue. It's true that "madExcept.SendBugReport" create a new instance of IMESettings, but that function is only exported for external use, it's not actually used by madExcept itself. All internal callers of SendBugReportEx() should use the current exception interface, so your changes to exceptIntf.MailFrom should fully work.
madshi
Site Admin
 
Posts: 9268
Joined: Sun Mar 21, 2004 5:25 pm

Re: Can not change reporting user name

Postby slemmnord » Tue Jan 31, 2017 5:07 am

Maybe the reason was as simple as copy-and-paste (from SMTP handler)? :P
I find it very hard to believe that web server would reject such fields (it would break the web). Maybe that user was extra paranoid and had configured the server such or maybe it's some framework which places such requirement.
Anyway, in our case it's up to the php script how to parse it. The example php script you have hosted tries to parse name and email out from "MailFrom" variable so i find no reason why ME on client-side should prevent adding that name part.

2nd issue:
Maybe i'm using the wrong method for sending? Here is what i use:

Code: Select all
procedure GlobalMadExceptionHandler(const exceptIntf: IMEException; var handled: Boolean);
var
  Wnd: HWND;
  Rslt: TModalResult;
  LSend: Boolean;
begin

  Handled := False;

  // no point showing exception message when shutting down; just save bug report
  if IsOsShutdown then
  begin
    AutoSaveBugReport(exceptIntf.BugReport, exceptIntf);
    Exit;
  end;

  // add some extra info
  exceptIntf.BugReportHeader.Add('Client version', CompileDateStr);
  exceptIntf.BugReportHeader.Add('System name', ObjectName);
  exceptIntf.BugReportHeader.Add('Core version', GServerVersion);

  // always save when debug versions
  {$IFNDEF Release}
  AutoSaveBugReport(exceptIntf.BugReport, exceptIntf);
  {$ENDIF}

  // if we were not able to synchronize then we can't show the nice dialogs
  // because those depend on vcl
  if GetCurrentThreadId <> MainThreadID then Exit;

  Wnd := GetTopWindow;

  // a fancy task dialog for unexpected errors
  Handled := True;
  Rslt := mrOk;   // continue is default

    with TTaskDialog.Create(nil) do
    try

      Caption := LoadResString(@SMsgDlgError);
      Title := STR_UNUSUAL_FAULT;
      Text := exceptIntf.ExceptMessage;
      if not IsOsShutdown then VerificationText := STR_SEND_ERROR_REPORT;    // can't send when shutting down
      CustomMainIcon.Handle := LoadIcon(HInstance, ICON_BUG);
      MainIcon := tdiNone;
      Flags := [tfAllowDialogCancellation, tfPositionRelativeToWindow, tfUseHiconMain];

      CommonButtons := [];

      if not IsOsShutdown then
        with TTaskDialogButtonItem(Buttons.Add) do
        begin
          Caption := STR_RESTART;
          ModalResult := mrAbort;
        end;

      with TTaskDialogButtonItem(Buttons.Add) do
      begin
        Caption := STR_CONTINUE;
        ModalResult := mrOk;
        Default := True;
      end;

      if Execute(Wnd) then Rslt := ModalResult;
      LSend := tfVerificationFlagChecked in Flags;

    finally
      Free;
    end;

  // do it
//  if LSend then exceptIntf.SendBugReport(Wnd);
  if LSend and DLGSendAssistant.Execute(PARENT_TOP_WINDOW) then
  begin

    exceptIntf.BugReportHeader.Add('contact name', DLGSendAssistant.edName.Text);
    exceptIntf.BugReportHeader.Add('contact email', DLGSendAssistant.edEmail.Text);
    if DLGSendAssistant.mmoDescription.Text <> ''
      then exceptIntf.BugReportSections.Insert(0, 'user description', DLGSendAssistant.mmoDescription.Text);

    // note: MESettings.MailFrom is correct; changing exceptIntf.MailFrom has no effect
    if DLGSendAssistant.edEmail.Text <> ''
      then MESettings.MailFrom := Trim(Format('%s <%s>', [Trim(DLGSendAssistant.edName.Text), Trim(DLGSendAssistant.edEmail.Text)]))
      else MESettings.MailFrom := Trim(Format('%s <bug@domain.com>', [Trim(DLGSendAssistant.edName.Text)]));

    if SendBugReport(exceptIntf.BugReport, exceptIntf.ScreenShot) <> Yes then
      if
        TaskDialogWithFallback(
          PARENT_TOP_WINDOW,
          STR_REPORT_NOT_SENT,
          STR_REPORT_SAVE_AND_SEND,
          mtError,
          [mbOK, mbCancel]) = mrOk
      then begin
        exceptIntf.SaveBugReport;
      end;
  end;

  if Rslt = mrAbort then exceptIntf.RestartApplication(Wnd);

end;
slemmnord
 
Posts: 19
Joined: Fri Jan 17, 2014 1:04 am

Re: Can not change reporting user name

Postby madshi » Tue Jan 31, 2017 8:32 am

I've found an email from a user. Here it is:

I have setup tracing on the IIS server and found that the failing request contains a couple of issues

1. The “From” HTTP header is badly formed

Test No | Contact values set | Status| HTTP Headers
1 | NOT SET | Successful | “From” header not set
2 | NAME set only | Successful | “From” header not set
3 | NAME AND EMAIL set | FAILED | “From” header set badly
4 | EMAIL set only | Successful | “From” header set OK

When the contact email is provided you set the HTTP From Header (As per test 4)

Headers="Connection: Keep-Alive
Content-Length: 10663
Content-Type: multipart/form-data; boundary=www.madshi.net_multipart_boundary
From: <cameron@whatever.com> <<NOTE THIS IS OK>>
Host: bug.something.com
User-Agent: madExcept/4.0

However if BOTH the name and email is provided you set the From header (As per test 3)

Headers="Connection: Keep-Alive
Content-Length: 10132
Content-Type: multipart/form-data; boundary=www.madshi.net_multipart_boundary
From: cameron <cameron@whatever.com> <<NOTE THIS IS BADLY FORMED>>
Host: bug.something.com
User-Agent: madExcept/4.0

The HTTP From header should just be the email address. I imagine IIS is being more particular about the FROM header than Apache is…

2. Even if you remove the FROM header it then fails on a potentially bad Form value (MailFrom). This value has the email in angle brackets which IIS detects could be a script hack.

The server response is

<span><H1>Server Error in '/Report' Application.<hr width=100% size=1 color=silver></H1>

<h2> <i>A potentially dangerous Request.Form value was detected from the client (MailFrom=&quot;...rmon hart &lt;cameron@whatever.com&gt;&quot;).</i> </h2></span>

<font face="Arial, Helvetica, Geneva, SunSans-Regular, sans-serif ">

<b> Description: </b>ASP.NET has detected data in the request that is potentially dangerous because it might include HTML markup or script. The data might represent an attempt to compromise the security of your application, such as a cross-site scripting attack. If this type of input is appropriate in your application, you can include code in a web page to explicitly allow it. For more information, see http://go.microsoft.com/fwlink/?LinkID=212874.
<br><br>

<b> Exception Details: </b>System.Web.HttpRequestValidationException: A potentially dangerous Request.Form value was detected from the client (MailFrom=&quot;...rmon &lt;cameron@whatever.com&gt;&quot;).<br><br>

As you can see, there's a good reason for what madExcept does.

However, there's a workaround for you, if you insist:

You can set "madExcept.HttpUploadCleanEmail := false". If you do that, "FROM:" will still be unchanged, but the private "MailFrom:" field will then contain the full email address + name.

About your 2nd issue: Your code is ok, but if you want your changes to "exceptIntf" to take effect for bug report sending, you need to feed "exceptIntf" to "SendBugReport()" as the 4th parameter.
madshi
Site Admin
 
Posts: 9268
Joined: Sun Mar 21, 2004 5:25 pm

Re: Can not change reporting user name

Postby slemmnord » Wed Feb 01, 2017 6:17 am

From that e-mail i take two points:
- as i had assumed, it is a framework (ASP.NET) that is being careful in default settings, not HTTP server
- the problem description clearly explains the reason for disallowing by default and that there are legitimate reasons/ways to allow it (this case is one)

I think ASP.NET default setting is a reasonable way to bring a web app developers attention to the fact that all input parameters should get checked and sanitized. But that's it. The user should have made the appropriate adjustments to his script.

While checking out what HttpUploadCleanEmail does i noticed this MailFrom change is recent and only since v4.0.14. Now looking at older reports i see they did come with names before. Perhaps this flag should be false by default to not break old behavior?

2nd issue: thanks! somehow i had not connected that IMEException is descendant of IMESettings.
slemmnord
 
Posts: 19
Joined: Fri Jan 17, 2014 1:04 am

Re: Can not change reporting user name

Postby madshi » Wed Feb 01, 2017 9:00 am

Being careful might be limited to certain setups, but still the overall gist I get from this is that the "FROM:" field doesn't like email names to be used. And in my google search all the FROM: examples had no name in them, either, but just the email address itself, without "<" and ">".

For me it's important that mailing works without having to change some half documented options in madExcept. So I don't plan on changing the "HttpUploadCleanEmail" default. If a new user tries uploading bug reports and it fails, it will be difficult to find out that he has to change "HttpUploadCleanEmail" to make it work. So changing the default value could cause a lot of extra support work.
madshi
Site Admin
 
Posts: 9268
Joined: Sun Mar 21, 2004 5:25 pm

Re: Can not change reporting user name

Postby slemmnord » Wed Feb 01, 2017 10:22 am

Umm.. "From"? ... ok, i see now i have made a mistake in my first post! Sorry about that. Actually the "From" field is not important at all. As far as the php script goes, it could even be skipped -- the script doesn't check for that field anywhere.

It's all about the "MailFrom" field. And that too is relevant only as far as the php script. How the script handles it (or what the field is called) is completely open-ended . The php script doesn't even process a field called "From". It fills the phpmailer classes "From" and "FromName" properties via "MailFrom" (tries to parse out both name and email and place in different fields). But now, by default, ME doesn't even send the "MailFrom" field any more. So all e-mails come from whatever default was set by the php script ($from_email = 'sender@email.com'; $from_name = 'sender name';).

Think about it -- It's as if you're saying HTTP POST method cant send <> characters. That makes no sense. Then we could not send bugreport.txt either because that is likely to contain <>.

I have the same motive in reporting and now pushing you a bit -- to get it working by default for most cases (i fixed it for me already).

Another alternative is to keep sender name and address in two different fields and adjust the php script accordingly. But there really is no good reason for that. And that would require everyone who's using the post method to update scripts again.

I think i've said all there is to say about the case. Thanks for listening.
slemmnord
 
Posts: 19
Joined: Fri Jan 17, 2014 1:04 am

Re: Can not change reporting user name

Postby madshi » Wed Feb 01, 2017 10:35 am

slemmnord wrote:But now, by default, ME doesn't even send the "MailFrom" field any more.

Not true.
madshi
Site Admin
 
Posts: 9268
Joined: Sun Mar 21, 2004 5:25 pm

Re: Can not change reporting user name

Postby slemmnord » Wed Feb 01, 2017 9:16 pm

Indeed, it also gets stripped down to address only. I was too tired to test and wrote from memory.
But that does not invalidate the rest of the argument.
slemmnord
 
Posts: 19
Joined: Fri Jan 17, 2014 1:04 am

Re: Can not change reporting user name

Postby madshi » Wed Feb 01, 2017 9:38 pm

I see two options:

1) I set "HttpUploadCleanEmail := true" by default. Result: Bug report sending works for everyone, but only the naked email address is reported.
2) I set "HttpUploadCleanEmail := false" by default. Result: Bug report sending will completely fail for some users, but when it works, the name is also reported.

I believe 1) is the better default option.

That said, maybe I should transmit the name in a separate field and update the PHP script accordingly.
madshi
Site Admin
 
Posts: 9268
Joined: Sun Mar 21, 2004 5:25 pm


Return to madExcept

Who is online

Users browsing this forum: No registered users and 2 guests

cron