mantisbt-plugins / source-integration

Source control integration plugin framework for MantisBT, including support for Github, Gitlab, Bitbucket, Gitea, Gitweb, Cgit, Subversion, Mercurial and more
http://noswap.com/projects/source-integration/
MIT License
181 stars 130 forks source link

Importing an empty HgWeb repository causes lockup #269

Closed Intuity closed 6 years ago

Intuity commented 6 years ago

When running an import using SourceHgWeb, if it runs on an empty Hg repository it would lockup and PHP CPU usage hits 100%. This is because the '#HG changeset patch' string is never found, and the script just locks in a while loop.

dregad commented 6 years ago

Thanks for your contribution. I don't have an issue with your patch, and I don't actually use hgweb, but I'm curious about hgweb's output in your case. I did a quick test (see below), and the target string # HG changeset patch is present in the output.

$ hg init hgtest
$ cd hgtest
$ hg serve

Then

$ curl http://localhost:8000/raw-rev

# HG changeset patch
# User 
# Date 0 0
# Node ID 0000000000000000000000000000000000000000
Intuity commented 6 years ago

Hi, it's actually because its querying for a non-existent branch. The URL being queried is: https://localhost:8000/raw-rev/default

Which then returns:

$ curl http://localhost:8000/raw-rev/default

error: 00manifest.i@default: no match found

This then fails to match the '# HG changeset patch' and hence loops forever.

Potentially a check could be made for the error being flagged instead, but this guard catches this case and any other unexpected responses from Mercurial.

dregad commented 6 years ago

the URL being queried is: https://localhost:8000/raw-rev/default

Ah yes that makes sense, thanks for the feedback.

In that case, I think it would be worth triggering an error instead of return array (null, array());, or at least displaying some helpful information / error message before the return statement (like it's done when the commit already exists).

Intuity commented 6 years ago

I've added a message to give some indication that the repository might be empty. I don't believe that an error is the right way to go, as IT administrators may setup empty repositories for others and as part of this register the repository in Mantis. It's not an invalid operation to import an empty repository, but I agree it's worth flagging to the user that they might want to double check the outcome.

dregad commented 6 years ago

I don't believe that an error is the right way to go, as IT administrators may setup empty repositories for others and as part of this register the repository in Mantis

Agreed.

I agree it's worth flagging to the user that they might want to double check the outcome.

I have updated the warning message to include the HgWeb output, to make that more convenient for users. Please check if that works correctly, and let me know how it goes. If you don't have any objections or comments, I'll merge the PR.

dregad commented 6 years ago

@Intuity any feedback ?

Intuity commented 6 years ago

Apologies for the delay. Looks good, had the following output when I tested it.

Retrieving default... Unexpected HgWeb response (error: 00manifest.i@default: no match found) - repository may be empty.

Happy for this to be the final version.