#227 HttpUnitUtils.replaceEntities() never returns

[dlleary]

When passed a String containing an ampersand but no
semicolon,
com.meterware.httpunit.HttpUnitUtils.replaceEntities()
gets stuck in an infinite loop.

This happens, e.g., when attempting to parse a refresh
header via
WebResponse.getRefreshRequest() when the refresh URL
contains
an ampersand and no semicolon.

The problem is that line 258 of HttpUnitUtils.java rev
1.20:

if (semiColonIndex < 0) continue;

should instead read:

if (semiColonIndex < 0) break;

[jcloughley]

Logged In: YES
user_id=1179133

dlleary correctly states that HttpUnitUtils.replaceEntities()
never returns in issue 1063494. However, I wonder if his
suggested fix is correct. The current code has

if (semiColonIndex < 0) continue;

and he suggests changing this to:

if (semiColonIndex < 0) break;

However, in my code, I fixed this issue by incrementing i such
as:

if (semiColonIndex < 0)
{
i++;
continue;
}

I'm new to this stuff but for my purposes, this breaks the
infinite loop and it seems to do the search/replace fine.

[dlleary]

Logged In: YES
user_id=1156159

Incrementing i and continuing will work too, but this will
just loop through the
rest of the string looking for more ampersands unnecessarily.

Since the first ampersand was not followed by a semicolon,
neither will any
other ampersands that might be found thereafter. Hence it
is safe, and most
efficient, to just break out of the loop the first time no
semicolon is found.

But this does point out another bug. The assignment to i
should be made
outside the "amp" comparison block. Instead of:

if (entityName.equalsIgnoreCase( "amp" )) {
string = string.substring( 0, ampIndex ) +
'&' + string.substring( semiColonIndex+1 );
i = ampIndex + 1;
}

We should have:

if (entityName.equalsIgnoreCase( "amp" )) {
string = string.substring( 0, ampIndex ) +
'&' + string.substring( semiColonIndex+1 );
}
i = ampIndex + 1;

Otherwise, if the string contains an entity reference other
than "&amp;",
say, "&lt;", the routine will also never return.

[Russell Gold]

Logged In: YES
user_id=37920

The change is now in cvs and will be in the next build

[dlleary]

Logged In: YES
user_id=1156159

This fix seems not to have made it into rev 1.21 (httpunit
1.6.1).
To recap, the method should read:

static String replaceEntities( String string ) {
int i = 0;
int ampIndex;
while ((ampIndex = string.indexOf( '&', i )) >= 0) {
int semiColonIndex = string.indexOf( ';',
ampIndex+1 );
if (semiColonIndex < 0) break; // bug 1063494

String entityName = string.substring(
ampIndex+1, semiColonIndex );
if (entityName.equalsIgnoreCase( "amp" )) {
string = string.substring( 0, ampIndex ) +
'&' + string.substring( semiColonIndex+1 );
}
i = ampIndex + 1; // bug 1063494

}
return string;
}

[Russell Gold]

Logged In: YES
user_id=37920

It appears that I never actually committed these changes...

They should be there now.