Ticket #1392 (closed defeito: fixed)

Opened 9 years ago

Last modified 9 years ago

Caracteres # e - no assunto estraga o resultado da busca

Reported by: amuller Owned by: diogenesduarte
Priority: alta Milestone: Expresso 2.2
Component: ExpressoMail Version: branch 2.2
Severity: média Keywords: serialize -- busca carateres estraga assunto
Cc: WorkGroup:

Description

Quando o assunto ou outro campo do email contém caracteres # e - estraga o resultado da busca. Devido o serialize da busca ter sido implementado de outra forma.

Ticket re-criado por o anterior não ter sido implementado no arquivamento local.

Change History

comment:1 Changed 9 years ago by eduardoalex

  • Owner changed from amuller to diogenesduarte

Diógenes,

Favor testar e reportar.

comment:2 Changed 9 years ago by diogenesduarte

  • Status changed from new to closed
  • Resolution set to fixed

Senhores, testei aqui e esse erro não acontece. O que devo fazer? coloco como inválido ou como resolvido?

comment:3 follow-up: ↓ 4 Changed 9 years ago by niltonneto

É importante verificar se os commits envolvidos no ticket #1073 foram replicados para o branch 2.2. Diogenes, poderia fazer isso?

Obrigado.

comment:4 in reply to: ↑ 3 Changed 9 years ago by diogenesduarte

Cara, tem algo estranho... A revisão [2671] que teoricamente resolveu o problema no ticket #1073 não está implementada completamente no branch 2.2. Digo isso pois apesar de alguns códigos da mudança estarem lá, a maioria absoluta não está.

Por outro lado, tentei reproduzir novamente esse erro no branch 2.2 e não consegui. Não seria o caso de quem fechou o outro ticket #1073 avaliar com calma e se for o caso reaplicar no branch 2.2?

Replying to niltonneto:

É importante verificar se os commits envolvidos no ticket #1073 foram replicados para o branch 2.2. Diogenes, poderia fazer isso?

Obrigado.

comment:5 Changed 9 years ago by amuller

  • Status changed from closed to reopened
  • Resolution fixed deleted

Quem fechou este ticket se equivocou.

O problema ainda acontece e vai acontecer enquanto aquele código que faz splits por --- e ### ainda estiver presente no Expresso. Existiu uma revisão com código que reescrevia os serializes colocando-os no PHP, como deveria ser, e retirando aqueles serializes feitos na munheca com splits de # e -.

Este código foi retirado (revertido) #1312 e este problema voltou, então não fechem este ticket porque o problema voltou a acontecer e vai acontecer até que seja refeito os splits da busca de alguma forma menos "maluca"

comment:6 Changed 9 years ago by rodsouza

A reversão do SVN em questão foi discutida e como ressaltei em  http://trac.expressolivre.org/ticket/1312#comment:37 foi um tiro no pé.

Problemas foram criados e muitas correções foram jogadas fora, situações como essa estão tornando-se rotineiras...

comment:7 Changed 9 years ago by niltonneto

  • Priority changed from normal to alta

Muller, concordo plenamente com você. Lembro que ano passado isso foi corrigido na versão 2.0, e foi reportado por usuários. A questão é que realmente no branch 2.2, essa correção não foi replicada como deveria.

Como sempre digo, cuidado com os merges pessoal....

comment:8 Changed 9 years ago by niltonneto

Muller e demais:

Este problema está sendo ocasionado pela implementação feita no ticket #1312. O responsável pela revisão [3380] deve rever a implementação ou revertê-la URGENTE!

comment:9 Changed 9 years ago by rodsouza

Como alertado ( http://trac.expressolivre.org/ticket/1312#comment:37) anteriormente, ainda quando a modificação foi realizada, os resultado de tal trariam problemas diversos.

Aqui está um deles!

Independente da origem, situações como a presente tornam-se cada vez mais comum, e o retrabalho envolvido e o tempo perdido imensurável.

comment:10 Changed 9 years ago by diogenesduarte

Só refrescando a memória, o ticket #1312 foi gerado pois o ticket #428 (na revisão [3223]) foi fechado desconsiderando que alguém algum dia alguém pudesse fazer uma busca em mensagens locais. Realmente quem não usa mensagens locais não teve problema nenhum, mas e quem usa? Reverter a revisão desse ticket não deveria nem ser discutido, não há coerência alguma em manter algo que apagou a busca em mensagens locais(O código referente a isso foi removido), a menos que estejamos pensando em nosso próprio umbigo.

Quanto ao problema narrado nesse ticket, é como já foi dito antes na imensa thread de discussão do ticket #1312: Isso ocorreu pois foi feita uma correção(ou várias) e uma melhoria em uma mesma revisão. A idéia é que os commits sejam atômicos, no sentido de cada revisão ser referente a um ticket, porém como isso não aconteceu, deu no que deu. Foram revertidas sabe-se lá quantas alterações de correção que deveriam ser mantidas em detrimento de sua disponibilização ter sido feita na mesma revisão de uma modificação de melhoria que resolvemos, após imensa discussão, reverter.

A gente pode continuar lamentando o que foi feito, ou resolver o problema, que não é um dos mais complicados, sobretudo para quem já fez isso antes...

comment:11 Changed 9 years ago by rodsouza

E por que as tão importantes mensagens locais não funcionam a contento até hoje? E ainda por que as mesmas causam tanto problemas?

comment:12 follow-up: ↓ 14 Changed 9 years ago by niltonneto

Diogenes, realmente ficar lamentando e divagando sobre o leite derramado não adianta. Por isso escrevi para o responsável da revisão [3380] rever a implementação ou revertê-la. Abraço!

comment:13 Changed 9 years ago by niltonneto

Ah, eu até poderia pedir para o Muller rever a implementação dele, e corrigir o problema do "---" na busca das mensagens locais, mas ele não trabalha aqui mais na Celepar.

comment:14 in reply to: ↑ 12 Changed 9 years ago by diogenesduarte

Fechado então. Creio que esse problema não é muito complicado resolver não...

Replying to niltonneto:

Diogenes, realmente ficar lamentando e divagando sobre o leite derramado não adianta. Por isso escrevi para o responsável da revisão [3380] rever a implementação ou revertê-la. Abraço!

comment:15 Changed 9 years ago by amuller

Vou recapitular os fatos:

1 - A busca usava uma implementação complicada e inadequada. 2 - Em seguida a busca das mensagens locais foram implementados sob esta implementação complicada. 3 - Foi refeito a busca de um jeito melhor. 4 - A busca local parou de funcionar por conta disso. 5 - Foi revertido a melhoria na busca por conta das mensagens locais.

Agora vou fazer alguns questionamentos:

1 - Não seria o caso da busca das mensagens locais não terem sido feitas em cima de um código errado?

2 - É melhor arrumar as mensagens locais ou estragar tudo devolta?

3 - Porque uma coisa "local", interfere tanto nas implementações do servidor php que não deveria a princípio nem "saber" que existe coisas locais!?

comment:16 follow-up: ↓ 17 Changed 9 years ago by niltonneto

Muller, sejamos práticos. O Diógenes se comprometeu em corrigir esse problema. E já aproveitando, quero reforçar que a correção deverá ser refeita a partir da reversão da revisão [3380], apenas corrigindo para que a pesquisa das mensagens locais funcionem.

comment:17 in reply to: ↑ 16 Changed 9 years ago by diogenesduarte

Então, o que eu estou fazendo é resolvendo o problema desse ticket. A reversão da [3380] foi o Bruno quem fez e eu não sei para qual versão foi retornada.

Quanto a esse problema da "#" e "-" no assunto gerar erro na busca, já descobri aqui a parte problemática e já estou trabalhando pra resolver isso. Hoje pela tarde acredito que já commito a correção.

Replying to niltonneto:

Muller, sejamos práticos. O Diógenes se comprometeu em corrigir esse problema. E já aproveitando, quero reforçar que a correção deverá ser refeita a partir da reversão da revisão [3380], apenas corrigindo para que a pesquisa das mensagens locais funcionem.

comment:18 Changed 9 years ago by diogenesduarte

Corrigido em [3722].

O que foi feito na revisão acima foi bem simples: usamos um rawurlencode no código de busca do php e no javascript fizemos o decode para a parte de assunto. Isso resolveu o problema do ##. Quanto ao problema dos caracteres --, o código já tinha um tratamento para isso, mas tinha um if(false) que o bloqueava. Checamos o history do svn e constatamos que ele nunca teve esse bloqueio. Verificamos ainda que esse tratamento, podia ser utilizado (e era mais seguro) para qualquer situação, com apenas uma pequena adaptação. Foi o que fizemos.

Fizemos ainda uma modificação na busca por mensagens locais, para também "encodar" o caracter # caso venha no assunto, e tivemos também que fazer uma modificação no retorno da data, pois o array do header modificado, porém em mensagens locais não foi, fazendo com que o tratamento para o caracter -- deixasse de funcionar.

Obs: O problema desse ticket foi resolvido, mas a forma de se fazer a busca continua com a forma complicada de vários splits. Obs2: Como esse ticket é problemático, vou demorar um pouco para fechar, para caso vocês achem algum problema nós possamos tentar resolver sem precisar reabrir o ticket. Já fiz diversos testes aqui e tudo funcionou muito bem.

comment:19 follow-up: ↓ 20 Changed 9 years ago by niltonneto

Diogenes, a implementação correta é aquela que, ao invés de utilizar "---" para separar as informações do resultado, usa-se um array conforme abaixo:

Ao invés de:

$ret_msg = $this->decode_string($from) . "--" . $subject . "--". gmdate("d/m/Y",$header ->udate)."--". $this->size_msg($header->Size) ."--". $flag;

Deve usar assim:

$ret_msg['from'] = $this->decode_string($from);  
$ret_msg['subject'] = $subject;  
$ret_msg['udate'] = $header ->udate;  
$ret_msg['size'] = $header->Size;  
$ret_msg['flag'] = $flag;  

comment:20 in reply to: ↑ 19 Changed 9 years ago by diogenesduarte

concordo plenamente, só não fiz isso meu foco foi resolver o problema narrado no ticket. Seria realmente interessante reavaliar toda a implementação dessa busca, pois não só esse split como outros pequenos detalhes seriam interessantes serem melhorados, mas eu acho que isso seria uma melhoria e deveria ser tratado em outro ticket, estou certo?

Replying to niltonneto:

Diogenes, a implementação correta é aquela que, ao invés de utilizar "---" para separar as informações do resultado, usa-se um array conforme abaixo:

Ao invés de:

$ret_msg = $this->decode_string($from) . "--" . $subject . "--". gmdate("d/m/Y",$header ->udate)."--". $this->size_msg($header->Size) ."--". $flag;

Deve usar assim:

$ret_msg['from'] = $this->decode_string($from);  
$ret_msg['subject'] = $subject;  
$ret_msg['udate'] = $header ->udate;  
$ret_msg['size'] = $header->Size;  
$ret_msg['flag'] = $flag;  

comment:21 Changed 9 years ago by niltonneto

As pesquisas que contêm palavras acentuadas ainda não retornam nada. Esta última revisão deveria resolver?

comment:22 Changed 9 years ago by diogenesduarte

Não, não mexi em nada em relação a isso. Vi um ticket para resolver esse problema, o #1485, não abri ela pois já tinha um owner.

comment:23 Changed 9 years ago by diogenesduarte

  • Status changed from reopened to closed
  • Resolution set to fixed

Como não teve nenhum comentário sobre essa correção, estou fechando esse ticket.

Note: See TracTickets for help on using tickets.