I got a few small changes to the ticket system, which I grouped into the branch tkt-improvements.

The changes contain:

• I got annoyed changing things in /tktsetup for testing purposes again and again and having to click through to it. To improve I added a submenu to the sub-pages of /tkt_setup that goes back to it, and also made 'Cancel' consistently go to the parent page. (fossil:f5619ef3e0bfa164)
• The old ticket fossil:3bcacd36ff9e8820 asked for showing the discussion so far on the /tktedit page, which I added. (fossil:ff009a8f0e482a86)
• In ticket reports, if you enable multiple rows per ticket (which can be done by naming field with leading underscore), then an empty row is added for each ticket, which is intended as a separator (as indicated by code comment). I changed it to contain a <hr>, so that it is visibly clearer to understand as separator. (fossil:7b08586a41d11f19)
• Ticket fossil:04ba0f70b816cebe asks for linking the version field in /tktview to the respective version. I added TH1-code to the default template, that checks if the content is either a valid tagname or a valid checkin-hash, and then links to it accordingly. (fossil:44bd394f0a618264)
  • EDIT: With the fixes for the injection issue that code now also checks the version-string before even attempting to query the DB. For checkin-hashes that looks for values containing only numbers and a-f, for tags alphanumeric values and underscores (_), dots (.) and hyphen (-). Values in the field not matching these will not get a link.
• Ticket fossil:14bc0ad52d919860 asks to show creation time as well. I am not aware if back at the time the ticket was created the tkt_ctime field already existed, but it does now. So I added it to the default template for new ticket reports and made the column names more clear (fossil:86c1910ffbd1b642), added the creation time to /tktview (fossil:814a417b4d2d1071) and also added human readable ages for both modification time and creation time to /tktview (fossil:8b48a054edda1a17).
• The submenu to delete a ticket report in /rptedit wasn't working. That is because of the CSRF-check (which is a good thing). I removed the submenu to avoid confusion. The functionality to delete a report is still available if you click the button in the form. (fossil:7c91416166a7ecbb)

As these are different things covered, it might be prudent to remove some of these before merging if you object to a specific thing in the list. I just bundled them in one branch, as each seems to small to justify it's own branch.

All of these changes are on the small side, but I think they all add a little quality of life to the ticket system to justify their inclusion.

I tried to implement as much as possible in changeable templates. This means that a lot of these things (like creation date or linking the version field) are customizable for the repository admins. It also means a lot will not show up in repositories until templates are reverted to default.

What do you think, are these changes fit for inclusion in trunk?

If there are no objection I would go on and merge this.

Please adjust source code files for a maximum line length of 80 characters.

I think I fixed all occurences of longer lines I introduced.

SQL Injection - DO NOT MERGE

SQL injection on the $foundin variable. For example, enter a new ticket where the first character of the "Version:" entry is single-quote (U+0027).

Oh, damn. I'll look into it or if I cannot fix it drop that change (which links the foundin-field to appropriate checkins). But let's see if I can catch it first. Thanks for the hint.

Thanks for the fix of the SQL injection. I added quoting for HTML and URL.

In my test repository I created some very wild tags to test this and found no problem after both our changes.

Also I put up a guard that I only call the query if the string matches a whitelist of characters. I wasted a bit of time here with string match, which isn't sufficient. But regexp does work. This guard means my wild tags aren't working anymore, as the code doesn't even look for such a tag, but I think that is fine as few will put in such characters in their tags and this only means that no link will be generated. Also this is only the default TH1 for the ticket-view, admins can change it at their will to allow for more stuff.

The only thing that bugs me, is that I could not properly quote the minus ('-') for the regexp in TH1, so that this isn't allowed in tags for links to be created as well (I allowed for numbers, letters, dot '.' and underscore '_', which I assume may turn up in tags representing versions).

EDIT: Got the minus added to the pattern, but I only managed via unicode escape.

Thanks. I managed with Unicode-escape, but this is the better solution as the unicode escape hides what is happening (except you memorized character codes, which I don't).

Having a look at that code, I was confused by the \ before [ ] $. Which are probably part of the TH1-escaping and not the regexp itself. Maybe having the pattern in { } is more readable - certainly less escape-y - but someone with profound TH1/TCL knowledge should judge:

set tagpattern {^[-0-9A-Za-z_\\.]+$}

(already contains the hyphen upfront, btw.)

Thanks alot!

I never in my life used TCL, so coming to fossil and having to deal with TH1 is a challenge for me. Sorry for my shortcomings. And thanks for these hints. This quoting makes indeed the intent of the regexp much more clear. I included that in the branch.

The branch now contains fixes for the injection issue. Are there other concerns?

If no objections are raised I would again consider merging into trunk. So please stop me if you see problems.

The branch now contains fixes for the injection issue. Are there other concerns?

FWIW, none here.