Empty strings in switch-statements #836

Closed
qnet-herwin opened this Issue Nov 19, 2014 · 6 comments

Projects

None yet

3 participants

@qnet-herwin
Contributor

Imagine the following config:

switch "%{Some-Attribute}" {
  case 'foo' { ... }
  case '' { ... }
  case { ... }
}

The intention of this statement is as following: in the case Some-Attribute equals 'foo', we want the first statement. In case there is no value (thus expanded to an empty string), we want the second statement. If there is a value other than foo, we want the default statement.

The current implementation sees case '' {} and case {} both as the default statement, which generates the message '"case" sections must have a name'.

The current workaround is to wrap it inside an if-statement:

if (!Some-Attributes) { ... } else { case .... }

If the empty string could be supported in the switch-statement, configs could be written a bit cleaner. I guess this is more like a feature request than a bug report.

@alandekok
Member

That config seems wrong on multiple levels.

switch Attribute-Name { 

is preferred to

switch "%{Attribute-Name}" {

Also, attributes should (in general) have content. Empty attributes are bad. Just omit the attribute instead.

But... it should allow for empty strings in case statements. I'll take a look

@alandekok alandekok closed this in dccf6b2 Nov 19, 2014
@qnet-herwin
Contributor

The example was just a simplication, the config where I've found this also included a %{tolower:...}.

The syntax here is still a bit tricky. If the attribute is not available, and the test is just simply for the attribute, the result is not an empty string but some undefined/null value. This doesn't equal an empty string (and shouldn't), so it will still fall back to the default.

I've got it working for a simple attribute without to-lower that uses %{unescape:%{escape:%{Some-Attribute}}}, but that syntax is a bit bizarre.

Honestly, I wouldn't know what the best solution to this would be. A possibility like case NULL { ... } does not fit in the framework.

@alandekok
Member

But %{tolower:...} just modifies an input string, So if the output is empty, it's because the input is empty.

I've pushed a fix. I've added a testcase for this situation. It should now work.

@qnet-herwin
Contributor

Not really what I meant. Let's just give an example config to illustrate what I was trying to say:

  update request {
    &Tmp-String-1 !* ANY
  }
  switch "%{tolower:%{request:Tmp-String-1}}" {
    case "" {
      update request {
        &Tmp-String-2 := "a"
      }
    }
    case {
      update request {
        &Tmp-String-2 := "b"
      }
    }
  }
  switch &request:Tmp-String-1 {
    case "" {
      update request {
        &Tmp-String-3 := "a"
      }
    }
    case {
      update request {
        &Tmp-String-3 := "b"
      }
    }
  }
  debug_request

generates the following debug output:

(0)         &request:Tmp-String-2 := a
(0)         &request:Tmp-String-3 := b

I would expect these two operations to give the same result.

@alandekok
Member

Those two operations won't give the same result. They're not intended to give the same result

When a non-existent attribute is expanded to a string "%{Tmp-Foo}", the result is a zero-length string.

When a non-existent attribute is referenced &Tmp-Foo, the result is no match.

@arr2036
Member
arr2036 commented Dec 8, 2014

Yes, absolutely, they're completely different operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment