idReplyPOP3.pas SetFormattedReply returns blank error response in idTCPConnection.TIdTCPConnection.GetInternalResponse about indy HOT 6 CLOSED

Can you please provide the complete unparsed response from the server? Preferably from a capture of the underlying socket communication, which you can get by assigning a TIdLog... component to the TIdPOP3.Intercept property. Or, at least, can you provide the actual contents of LResponse?

The parsing of LResponse's data is handled by TIdReplyPOP3.SetFormattedReply(), and that code hasn't been changed in a decade without problems. What are the contents of FLastCmdResult.Code and FLastCmdResult.Text after the assignment to FLastCmdResult.FormattedReply? Are they correct, at least?

Reading from FLastCmdResult.FormattedReply is going to regenerate a new reply based on the current FLastCmdResult.Code, TIdReplyPOP3(FLastCmdResult).EnhancedCode, and FLastCmdResult.Text values, so of course I would expect it to potentially be different than the server's original response. But, it should not be blank, either. The only way the result would be completely blank is if FLastCmdResult.Code and FLastCmdResult.Text are both blank to begin with, which would imply either TIdReplyPOP3.SetFormattedReply() has a bug, or the server's response is malformed to begin with.

from indy.

The contents of LResponse in GetInternalResponse() is:

'TLS 1.0 and 1.1 are not supported. Please upgrade/update your client to support TLS 1.2. Visit https://aka.ms/popimap_tls.'#$D#$A

LLine is:

'TLS 1.0 and 1.1 are not supported. Please upgrade/update your client to support TLS 1.2. Visit https://aka.ms/popimap_tls.'

Unfortunately, the TIdLog doesn't appear to be logging anything. I think the error is occurring prior to it's execution.

But - I'm hoping I can go one better.

I think if you create a new project and run this code and you should be able to duplicate what I'm seeing:

var
  POP3: TIdPOP3;
  IOHandler: TIdSSLIOHandlerSocketOpenSSL;
begin
  Pop3 := TidPOP3.Create(self);
  iohandler := TIdSSLIOHandlerSocketOpenSSL.create(self);

  pop3.IOHandler := IOHandler;
  pop3.UseTLS := utUseImplicitTLS;

  iohandler.SSLOptions.Method := sslvTLSv1;

  pop3.Host := 'outlook.office365.com';
  pop3.Password := 'anything'; //Don't care - we won't get this far to use it
  pop3.Username := '[email protected]'; //Don't care - we won't get this far to use it
  pop3.Port := 995;

  pop3.connect;
end;

As you mentioned, and I noticed that this code hasn't been touched in over a decade - I think it might be Microsoft once again breaking standards and doing their own thing.

Thanks for taking the time to look at this!

from indy.

The contents of LResponse in GetInternalResponse() is:

Is that everything in it? If so, then that response does not conform to the POP3 protocol as there is no POP3 status code present at the front of it. TIdReplyPOP3.SetFormattedReply() is going to be looking for that status code, and will end up treating the initial 'TLS' word as the status code, which is not a valid POP3 status code (only '+OK', '-ERR', and '+' are allowed). So, TIdReplyPOP3.SetFormattedReply() should be setting FLastCmdResult.Code to '-ERR' as a fallback, and should be adding the rest of the message ('1.0 and 1.1 are not supported. Please upgrade/update your client to support TLS 1.2. Visit https://aka.ms/popimap_tls.') to FLastCmdResult.Text.

Unfortunately the TIdLog doesn't appear to be logging anything. I think the error is occurring prior to it's execution.

It does not. The log will record all of the raw data that the IOHandler reads/writes, before the error occurs. You likely assigned the logger but didn't configure/activate it.

I think if you create a new project and run this code and you should be able to duplicate what I'm seeing:

Unfortunately, that is not an option for me at the moment, but I can follow along with what the source code is actually doing.

iohandler.SSLOptions.Method := sslvTLSv1;

You are telling the SSLIOHandler to use TLS 1.0, which the error message clearly says is not supported by the server. You need to use sslvTLSv1_2 instead to use TLS 1.2.

from indy.

Hi Remy,

Thanks again for your reply. Got the debug working - stupid error on my behalf.

This is the full RAW log file (using TidLogFile component):

Stat Connected.
Recv 28/04/2023 11:55:03 AM: TLS 1.0 and 1.1 are not supported. Please upgrade/update your client to support TLS 1.2. Visit https://aka.ms/popimap_tls.<EOL>
Stat Disconnected.

I'm aware that the server only supports TLS1.2, and that is the fix. My concern is that clients who haven't selected the right security are getting a blank error message returned as opposed to the message the server is responding, and that future error messages spat out like this but for different reasons won't be passed along.

I'm fairly convinced that the message does not conform to POP3 protocols - but this is Microsoft we're talking about. They make up their own rules, so I doubt this is going to change, and honestly - they've done a lot in the last 12 months that's out of the ordinary. (AKA - the changes we had to implement to oAuth because they're doing that wrong too) - so I have to find a way of dealing with them.

I think I've found where the problem is occurring, and it's deeper than idPOP3. It's in idReply.SetCode.

In SetFormattedReply, It calls

    if LOrd = -1 then begin
      Code := ST_ERR;
      end

This inturn calls TIdReply.SetCode(const AValue: string) which has a CLEAR call in it that's being executed.

I could put in an if statement here so that it only executes the clear if avalue <> -ERR

    if avalue <> '-ERR' then  //Modification made
     Clear;

Here's where I'm getting hesitant to start changing things. Given this is done at IDReply level that will affect a lot more than just POP3 responses, so I'm probably going to break something else with my fix.

Alternatively I could add something back at the POP3 level in TIdReplyPOP3.SetFormattedReply to work like:

    var oldtext := Text.Text;
    if LOrd = -1 then
    begin
      Code := ST_ERR;
      if text.text <> oldtext then
        text.text := OldText;
    end;

... which while messier, makes sure that it limits my changes only to POP3 when ST_ERR is being assigned because of an incorrectly formatted response. Short of there being a more correct solution (than expecting Microsoft to do things right) - I'm considering going with the latter option.

Best Regards

Hairy.

from indy.

This is the full RAW log file (using TidLogFile component):

Stat Connected.
Recv 28/04/2023 11:55:03 AM: TLS 1.0 and 1.1 are not supported. Please upgrade/update your client to support TLS 1.2. Visit https://aka.ms/popimap_tls.<EOL>
Stat Disconnected.

Thought so, just wanted to make sure.

I'm aware that the server only supports TLS1.2, and that is the fix. My concern is that clients who haven't selected the right security are getting a blank error message returned as opposed to the message the server is responding, and that future error messages spat out like this but for different reasons won't be passed along.

Understood.

I'm fairly convinced that the message does not conform to POP3 protocols - but this is Microsoft we're talking about. They make up their own rules, so I doubt this is going to change, and honestly - they've done a lot in the last 12 months that's out of the ordinary. (AKA - the changes we had to implement to oAuth because they're doing that wrong too) - so I have to find a way of dealing with them.

Stupid Microsoft.

I think I've found where the problem is occurring, and it's deeper than idPOP3. It's in idReply.SetCode.
...
This inturn calls TIdReply.SetCode(const AValue: string) which has a CLEAR call in it that's being executed.

Good catch!

I could put in an if statement here so that it only executes the clear if avalue <> -ERR

I think it would make more sense to have TIdReplyPOP3.SetFormattedReply() handle this at a higher level.

Here's where I'm getting hesitant to start changing things. Given this is done at IDReply level that will affect a lot more than just POP3 responses, so I'm probably going to break something else with my fix.

Exactly, which is why I don't like it, either.

Alternatively I could add something back at the POP3 level in TIdReplyPOP3.SetFormattedReply:

That's what I'm thinking, too.

It may make more sense to have TIdReplyPOP3.SetFormattedReply() check if the response begins with a proper POP3 status indicator, and if not then simply raise an EIdReplyPOP3Error exception immediately with -ERR as the Code and the whole response text as its Message (kind of like when TIdReplyPOP3.CheckIfCodeIsValid(), which TIdReply.SetCode() calls, is given an invalid status code, which in this situation it kind of is).

But, I'm OK with keeping the existing SetFormattedReply() logic in place, if it can work around the Clear() issue in a simple and clean manner. For instance, the simplest solution would be to just have TIdReplyPOP3.SetFormattedReply() assign that ST_ERR value to the inherited FCode member directly (like it does with the FEnhancedCode member) instead of invoking the TIdReply.SetCode() property setter. Otherwise, it could store the determined Code, EnhancedCode, and Text values into local variables first, then update the local Code if unassigned, then finally assign the local variables to the class properties when ready.

from indy.

Thanks so much Remy! Really appreciate your work and effort!

from indy.