Project

General

Profile

Actions

action #95842

closed

URLs containing "#" should be parsed as complete URLs size:S

Added by okurz about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Feature requests
Target version:
Start date:
2021-07-22
Due date:
2021-08-12
% Done:

0%

Estimated time:

Description

Observation

Putting an URL like https://build.suse.de/request/show/245055#comment-3760740 into an openQA comment field is converted into HTML code

also commented on <a href="https://build.suse.de/request/show/245055">https://build.suse.de/request/show/245055</a>#comment-3760740

also commented on https://build.suse.de/request/show/245055#comment-3760740

Expected result

should be

also commented on <a href="https://build.suse.de/request/show/245055#comment-3760740">https://build.suse.de/request/show/245055#comment-3760740</a>

also commented on https://build.suse.de/request/show/245055#comment-3760740

Acceptance criteria

  • AC1: The complete URL including the part after # is converted into an <a href>

Suggestions

Actions #1

Updated by VANASTASIADIS about 3 years ago

  • Assignee set to VANASTASIADIS
Actions #2

Updated by tinita about 3 years ago

  • Description updated (diff)
Actions #3

Updated by okurz about 3 years ago

  • Subject changed from URLs containing "#" should be parsed as complete URLs to URLs containing "#" should be parsed as complete URLs size:S
  • Status changed from New to Workable

estimated asynchronously by vanastasiadis and okurz

Actions #4

Updated by kraih about 3 years ago

Took a quick look at this since i wrote the Markdown conversion code. And i'm afraid it is not as easy as it looks at first. We are currently using a regex from Regexp::Common to detect URLs in OpenQA::Markdown. That regex will need to be replaced with one that includes fragments. This is a serious XSS/HTML injection attack target, so the regex will need to be selected very carefully. And new tests should cover as many edge cases as possible.

Actions #5

Updated by kraih about 3 years ago

This is the line you want to modify and the new tests go here.

Actions #6

Updated by VANASTASIADIS about 3 years ago

This line produces the same error as well. Is there a reason to leave it out?

Actions #7

Updated by VANASTASIADIS about 3 years ago

btw, with a quick look, the current implementations both in Markdown.pm and Utils.pm both seem to be already open to injection attacks. So I think it makes sense to change both.

Actions #8

Updated by okurz about 3 years ago

  • Due date set to 2021-08-12
  • Status changed from Workable to In Progress
Actions #9

Updated by VANASTASIADIS about 3 years ago

  • Status changed from In Progress to Resolved
Actions #10

Updated by VANASTASIADIS about 3 years ago

  • Status changed from Resolved to Feedback
Actions #11

Updated by VANASTASIADIS about 3 years ago

  • Status changed from Feedback to Resolved

Deployed. Resolving the ticket
https://openqa.suse.de/changelog

Actions

Also available in: Atom PDF