Porque é que este loop infinitamente?

Question

Então, eu só tenho o meu site expulso do servidor hoje e acho que esta função é o culpado. Alguém pode me dizer qual é o problema? Eu não consigo descobrir isso:

Public Function CleanText(ByVal str As String) As String    
'removes HTML tags and other characters that title tags and descriptions don't like
    If Not String.IsNullOrEmpty(str) Then
        'mini db of extended tags to get rid of
        Dim indexChars() As String = {"<a", "<img", "<input type=""hidden"" name=""tax""", "<input type=""hidden"" name=""handling""", "<span", "<p", "<ul", "<div", "<embed", "<object", "<param"}

        For i As Integer = 0 To indexChars.GetUpperBound(0) 'loop through indexchars array
            Dim indexOfInput As Integer = 0
            Do 'get rid of links
                indexOfInput = str.IndexOf(indexChars(i)) 'find instance of indexChar
                If indexOfInput <> -1 Then
                    Dim indexNextLeftBracket As Integer = str.IndexOf("<", indexOfInput) + 1
                    Dim indexRightBracket As Integer = str.IndexOf(">", indexOfInput) + 1
                    'check to make sure a right bracket hasn't been left off a tag
                    If indexNextLeftBracket > indexRightBracket Then 'normal case
                        str = str.Remove(indexOfInput, indexRightBracket - indexOfInput)
                    Else
                        'add the right bracket right before the next left bracket, just remove everything
                        'in the bad tag
                        str = str.Insert(indexNextLeftBracket - 1, ">")
                        indexRightBracket = str.IndexOf(">", indexOfInput) + 1
                        str = str.Remove(indexOfInput, indexRightBracket - indexOfInput)
                    End If
                End If
            Loop Until indexOfInput = -1
        Next
    End If
    Return str
End Function

Answer 1

Não seria algo como isto ser mais simples? (OK, eu sei que não é idêntico ao código postado):

public string StripHTMLTags(string text)
{
    return Regex.Replace(text, @"<(.|\n)*?>", string.Empty);
}

(A conversão para VB.NET deve ser trivial!)

Nota: se você estiver executando isso muitas vezes, há duas melhorias de desempenho que você pode fazer ao Regex.

Uma é usar uma expressão pré-compilado que requer re-escrever um pouco.

O segundo é a utilização de uma forma não-captura da expressão regular; .NET expressões regulares implementar o (? :) sintaxe, o que permite o agrupamento de ser feito sem incorrer na pena de texto capturado desempenho ser lembrado como referência anterior. Usando essa sintaxe, o acima expressão regular pode ser alterado para:

@"<(?:.|\n)*?>"

Answer 2

Esta linha também é errado:

Dim indexNextLeftBracket As Integer = str.IndexOf("<", indexOfInput) + 1

É garantido para sempre definido indexNextLeftBracket igual a indexOfInput, porque neste momento o caractere na posição referida por indexOfInput já é sempre um '<'. Faça isso em vez disso:

Dim indexNextLeftBracket As Integer = str.IndexOf("<", indexOfInput+1) + 1

E também adicionar uma cláusula para a instrução if para se certificar de sua seqüência é tempo suficiente para que a expressão.

Finalmente, como outros disseram que este código será uma besta de manter, se você pode fazê-lo funcionar em tudo. Melhor procurar outra solução, como um regex ou mesmo apenas a substituição de todos '<' com <.

Answer 3

Além de outras boas respostas, que você pode ler um pouco sobre invariantes de laço um pouco. O retirando e colocando coisas de volta para a cadeia de verificar de terminar o seu ciclo deve detonar todos os tipos de alarme. :)

Answer 4

Apenas um palpite, mas isso é como o culpado? indexOfInput = str.IndexOf (indexChars (i)) 'encontrar instância de indexChar

De acordo com os Microsoft docs , Return Valor - A posição do índice de valor que se corda encontra-se, ou -1, se não for. Se o valor estiver vazio, o valor de retorno é 0.

Assim, talvez indexOfInput está sendo definido para 0?

Answer 5

O que acontece se o seu código tenta limpar o <a corda?

Como eu li, ele encontra o indexChar na posição 0, mas depois indexNextLeftBracket e indexRightBracket ambos igual a 0, você cair na condição de pessoa, e, em seguida, inserir um ">" na posição -1, que, presumivelmente, inserir no o início, dando-lhe a ><a string. O novo indexRightBracket então se torna 0, portanto, de excluir da posição 0 para 0 caracteres, deixando-o com ><a. Em seguida, o código encontra a <a o código de novo, e você está pronto para as corridas com um loop consome memória infinita.

Mesmo se eu estiver errado, você precisa obter-se alguns testes de unidade para tranquilizar-se que estes casos extremos funcionar corretamente. Isso também deve ajudar a encontrar o código looping real se eu estiver off-base.

De um modo geral, porém, mesmo se você corrigir esse bug particular, ele nunca vai ser muito robusto. Análise de HTML é difícil, e blacklists HTML são sempre vai ter buracos. Por exemplo, se eu realmente quero ter uma tag <input type="hidden" name="tax" em, eu vou escrevê-lo como <input name="tax" type="hidden" e seu código irá ignorá-lo. Sua melhor aposta é obter um analisador HTML real envolvido, e para permitir que apenas o (muito pequeno) subconjunto de tags que você realmente quer. Ou melhor ainda, usar alguma outra forma de marcação, e tira todas as tags HTML (novamente usando um analisador HTML reais de alguma descrição).

Answer 6

Eu teria que executá-lo através de um compilador real, mas o mindpiler me diz que a linha str = str.Remove(indexOfInput, indexRightBracket - indexOfInput) é re-gerando uma tag inválida de tal forma que quando você percorrer novamente ele encontra o mesmo erro "correções" que, tenta novamente, encontra as erro "correções" que, etc.

FWIW aqui está um trecho de código que remove indesejadas tags HTML de uma string (é em C #, mas o conceito se traduz)

public static string RemoveTags( string html, params string[] allowList )
{
    if( html == null ) return null;
    Regex regex = new Regex( @"(?<Tag><(?<TagName>[a-z/]+)\S*?[^<]*?>)",
                             RegexOptions.Compiled | 
                             RegexOptions.IgnoreCase | 
                             RegexOptions.Multiline );
    return regex.Replace( 
                   html, 
                   new MatchEvaluator( 
                       new TagMatchEvaluator( allowList ).Replace ) );
}

classe MatchEvaluator

private class TagMatchEvaluator
{
    private readonly ArrayList _allowed = null;

    public TagMatchEvaluator( string[] allowList ) 
    { 
        _allowed = new ArrayList( allowList ); 
    }

    public string Replace( Match match )
    {
        if( _allowed.Contains( match.Groups[ "TagName" ].Value ) )
            return match.Value;
        return "";
    }
}

Answer 7

Isso não parece trabalho para um caso <a<a<a simplista, ou mesmo <a>Test</a>. Será que quis testar isso em tudo?

Pessoalmente, eu odeio análise de cadeia como este - então eu não vou mesmo tentar descobrir onde o seu erro é. Seria exigir um depurador, e mais dor de cabeça do que eu estou disposto a colocar em.