Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(validator): source_url with short host and required protocol #465

Merged
merged 2 commits into from
Jul 16, 2022

Conversation

aprendendofelipe
Copy link
Collaborator

  1. Resolve o problema de não aceitar URL curta como https://t.me. Problema relatado em Mais um bug no source_url sobre URL inválida #457

  2. O novo limite do tamanho do TLD passou de 18 para 24 caracteres e também são aceitos números e hífens, conforme a lista de TLDs.

  3. Torna obrigatória a inserção do protocolo (http ou https), pois sem isso o componente Content considera como URL relativa e cria um link inválido:
    Cadastrando www.google.com o link ficava como https://www.tabnews.com.br/user/www.google.com

  4. Agora não é permitido letras maiúsculas no host, apenas após o TLD (path, query e fragment). Antes era possível cadastrar algo como https://www.gOoGlE.com. Ainda é possível algo como https://www.tabnews.com.br/#:~:text=TabNews,-Status

Obs.: Optei por manter o regex e não utilizar o Joi, pois é mais fácil de entender quais regras estão sendo aplicadas.
Obs. 2: Me baseei na RFC 3986.

@vercel
Copy link

vercel bot commented Jun 20, 2022

@aprendendofelipe is attempting to deploy a commit to the TabNews Team on Vercel.

To accomplish this, @aprendendofelipe needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@aprendendofelipe
Copy link
Collaborator Author

@filipedeschamps como existe uma verificação dos conteúdos antes de criar as páginas estáticas, é preciso corrigir o source_url de algumas postagens antes do deploy.

Em homologação, é preciso corrigir o source_url desse post

https://tabnews-git-cache-user-state-tabnews.vercel.app/aismaniotto/teste

É preciso inserir o protocolo no início da URL, pois olha como ficou o link:

https://tabnews-git-cache-user-state-tabnews.vercel.app/aismaniotto/www.eu.mesmo.com.br

Em produção só tem essa postagem do Guga

https://www.tabnews.com.br/gugadeschamps/sistema-de-reconhecimento-facial-utilizado-por-bancos-para-prova-de-vida-sao-extremamente-vulneraveis-a-deepfakes

O título da página foi inserido no final da URL e o link só funciona provavelmente porque o theverge.com dá um jeito de reconhecer qual seria o caminho correto, mas isso não vai acontecer em qualquer página.

Obs. Talvez seja interessante retornar algo em caso de erro no validator nesse código dentro de authorization, pois assim não deixa de montar a página em caso de algum post que de alguma forma tenha ido para o BD sem passar pela validação

if (feature === 'read:content:list') {
    filteredOutputValues = output.map((content) => {
      return validator(content, {
        content: 'required',
      });
    });
  }

@filipedeschamps
Copy link
Owner

filipedeschamps commented Jun 20, 2022

@filipedeschamps como existe uma verificação dos conteúdos antes de criar as páginas estáticas, é preciso corrigir o source_url de algumas postagens antes do deploy.

Sensacional essa implementação @aprendendofelipe 😍 muito massa com o que um assunto simples pode acabar tendo tanta complexidade. Mas fico muito feliz que você está "lambedo" isso 🤝

Mas de qualquer forma, sugiro então fazermos toda essa movimentação no final da milestone assim como os outros PRs, daí a gente faz o update manual dos valores no banco e o merge do código.

Em paralelo, já antecipo que muita coisa nos testes pode quebrar com a branch das tabcoins, pois foi feita uma refatoração intensa para um assert mais strict (e isso fez pegar alguns bugs e falta de cobertura na interface).

Sugiro qualquer implementação feita nos testes a partir de agora seja naquele formato. Inclusive, não sei se é arriscado por agora, mas se em paralelo fazer o rebase contra aquela branch, vai evitar de tomarmos um susto no final da milestone 🤝

@aprendendofelipe
Copy link
Collaborator Author

Tranquilo @filipedeschamps, mas com as mudanças que ainda podem vir, acho melhor deixar o rebase para ser feito mais pra frente também 🤝

@aprendendofelipe
Copy link
Collaborator Author

@filipedeschamps, esse PR está pronto para ir para a homologação. 🚀

Só não pode esquecer de usar seus poderes de editar conteúdos de terceiros para corrigir o source_url de alguns posts (foi postado mais um desde a minha mensagem anterior sobre isso).

Em ambiente de homologação (adicionar http:// ou excluir esses source_url fakes):

Em produção (excluir texto presente após a url):

@filipedeschamps
Copy link
Owner

Perfeito meu caro! 😍

Vou começar a organizar o draft da Milestone e obrigado pelas URLs onde daria problema 🤝

@filipedeschamps
Copy link
Owner

@aprendendofelipe URLs consertadas! Partindo daqui a pouco para o deploy 👍

@aprendendofelipe
Copy link
Collaborator Author

@aprendendofelipe URLs consertadas! Partindo daqui a pouco para o deploy 👍

Boa! Acabei de testar aqui pra ver se não tinha nenhum conteúdo novo que daria problema, mas está tudo passando na regex sem problemas.

@vercel
Copy link

vercel bot commented Jul 16, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tabnews ✅ Ready (Inspect) Visit Preview Jul 16, 2022 at 11:06PM (UTC)

@filipedeschamps
Copy link
Owner

Só para confirmar o comportamento atual quebrado, eu testei na última URL de homologação https://tabnews-git-alterandofontetooltip-tabnews.vercel.app/ o endereço https://t.me/durov/186 e retorna erro. Em seguida vou fazer o deploy com o fix e testar de novo.

image

@filipedeschamps
Copy link
Owner

@filipedeschamps filipedeschamps merged commit 0e28ad5 into main Jul 16, 2022
@filipedeschamps filipedeschamps deleted the source-url-validator branch July 16, 2022 23:09
@filipedeschamps
Copy link
Owner

Merged! Let's goooooo!!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants