Page MenuHomeSoftware Heritage
Files F9312503

D8247.id29792.diff
No OneTemporary
Actions

D8247.id29792.diff
View Options

diff --git a/assets/src/bundles/add_forge/create-request.js b/assets/src/bundles/add_forge/create-request.js
--- a/assets/src/bundles/add_forge/create-request.js
+++ b/assets/src/bundles/add_forge/create-request.js
@@ -5,10 +5,12 @@
* See top-level LICENSE file for more information
*/
-import {handleFetchError, errorMessageFromResponse, csrfPost,
- getHumanReadableDate, genLink} from 'utils/functions';
-import userRequestsFilterCheckboxFn from 'utils/requests-filter-checkbox.ejs';
import {swhSpinnerSrc} from 'utils/constants';
+import {
+ csrfPost, errorMessageFromResponse, genLink, getHumanReadableDate,
+ handleFetchError, validateUrl
+} from 'utils/functions';
+import userRequestsFilterCheckboxFn from 'utils/requests-filter-checkbox.ejs';
let requestBrowseTable;
@@ -122,3 +124,11 @@
]
});
}
+
+export function validateForgeUrl(input) {
+ let customValidity = '';
+ if (!validateUrl(input.value.trim(), ['http', 'https'])) {
+ customValidity = 'The provided forge URL is not valid.';
+ }
+ input.setCustomValidity(customValidity);
+}
diff --git a/assets/src/bundles/save/index.js b/assets/src/bundles/save/index.js
--- a/assets/src/bundles/save/index.js
+++ b/assets/src/bundles/save/index.js
@@ -5,11 +5,13 @@
* See top-level LICENSE file for more information
*/
-import {csrfPost, handleFetchError, isGitRepoUrl, htmlAlert,
- getCanonicalOriginURL, getHumanReadableDate} from 'utils/functions';
import {swhSpinnerSrc} from 'utils/constants';
-import artifactFormRowTemplate from './artifact-form-row.ejs';
+import {
+ csrfPost, getCanonicalOriginURL, getHumanReadableDate, handleFetchError,
+ htmlAlert, isGitRepoUrl, validateUrl
+} from 'utils/functions';
import userRequestsFilterCheckboxFn from 'utils/requests-filter-checkbox.ejs';
+import artifactFormRowTemplate from './artifact-form-row.ejs';
let saveRequestsTable;
@@ -352,21 +354,11 @@
export function validateSaveOriginUrl(input) {
const originType = $('#swh-input-visit-type').val();
- let originUrl = null;
- let validUrl = true;
-
- try {
- originUrl = new URL(input.value.trim());
- } catch (TypeError) {
- validUrl = false;
- }
+ const allowedProtocols = ['http:', 'https:', 'svn:', 'git:', 'rsync:',
+ 'pserver:', 'ssh:', 'bzr:'];
+ const originUrl = validateUrl(input.value.trim(), allowedProtocols);
- if (validUrl) {
- const allowedProtocols = ['http:', 'https:', 'svn:', 'git:', 'rsync:', 'pserver:', 'ssh:', 'bzr:'];
- validUrl = (
- allowedProtocols.find(protocol => protocol === originUrl.protocol) !== undefined
- );
- }
+ let validUrl = originUrl !== null;
if (validUrl && originType === 'git') {
validUrl = isGitRepoUrl(originUrl);
diff --git a/assets/src/utils/functions.js b/assets/src/utils/functions.js
--- a/assets/src/utils/functions.js
+++ b/assets/src/utils/functions.js
@@ -96,17 +96,27 @@
return `<div class="alert alert-${type} ${extraClasses}" role="alert">${message}${closeButton}</div>`;
}
-export function isValidURL(string) {
+export function validateUrl(url, allowedProtocols = []) {
+ let originUrl = null;
+ let validUrl = true;
+
try {
- new URL(string);
- } catch (_) {
- return false;
+ originUrl = new URL(url);
+ } catch (TypeError) {
+ validUrl = false;
}
- return true;
+
+ if (validUrl && allowedProtocols.length) {
+ validUrl = (
+ allowedProtocols.find(protocol => protocol === originUrl.protocol) !== undefined
+ );
+ }
+
+ return validUrl ? originUrl : null;
}
export async function isArchivedOrigin(originPath) {
- if (!isValidURL(originPath)) {
+ if (!validateUrl(originPath)) {
// Not a valid URL, return immediately
return false;
} else {
diff --git a/cypress/e2e/add-forge-now-request-create.cy.js b/cypress/e2e/add-forge-now-request-create.cy.js
--- a/cypress/e2e/add-forge-now-request-create.cy.js
+++ b/cypress/e2e/add-forge-now-request-create.cy.js
@@ -43,7 +43,7 @@
cy.visit(this.addForgeNowUrl);
// create requests for the user 'user'
- populateForm('gitlab', 'gitlab.org', 'admin', 'admin@example.org', 'on', '');
+ populateForm('gitlab', 'https://gitlab.org', 'admin', 'admin@example.org', 'on', '');
cy.get('#requestCreateForm').submit();
// user requests filter checkbox should be in the DOM
@@ -62,9 +62,9 @@
cy.userLogin();
cy.visit(this.addForgeNowUrl);
- populateForm('gitea', 'gitea.org', 'admin', 'admin@example.org', 'on', '');
+ populateForm('gitea', 'https://gitea.org', 'admin', 'admin@example.org', 'on', '');
cy.get('#requestCreateForm').submit();
- populateForm('cgit', 'cgit.org', 'admin', 'admin@example.org', 'on', '');
+ populateForm('cgit', 'https://cgit.org', 'admin', 'admin@example.org', 'on', '');
cy.get('#requestCreateForm').submit();
// user requests filter checkbox should be in the DOM
@@ -208,7 +208,7 @@
it('should update browse list on successful submission', function() {
cy.userLogin();
cy.visit(this.addForgeNowUrl);
- populateForm('bitbucket', 'gitlab.com', 'test', 'test@example.com', 'on', 'test comment');
+ populateForm('bitbucket', 'https://gitlab.com', 'test', 'test@example.com', 'on', 'test comment');
cy.get('#requestCreateForm').submit();
cy.visit(this.addForgeNowUrl);
@@ -225,7 +225,7 @@
it('should show error message on conflict', function() {
cy.userLogin();
cy.visit(this.addForgeNowUrl);
- populateForm('bitbucket', 'gitlab.com', 'test', 'test@example.com', 'on', 'test comment');
+ populateForm('bitbucket', 'https://gitlab.com', 'test', 'test@example.com', 'on', 'test comment');
cy.get('#requestCreateForm').submit();
cy.get('#requestCreateForm').submit(); // Submitting the same data again
@@ -250,7 +250,7 @@
cy.visit(this.addForgeNowUrl);
populateForm(
- 'bitbucket', 'gitlab.com', 'test', 'test@example.com', 'off', 'comment'
+ 'bitbucket', 'https://gitlab.com', 'test', 'test@example.com', 'off', 'comment'
);
cy.get('#requestCreateForm').submit();
@@ -261,4 +261,16 @@
});
});
+ it('should bot validate form when forge URL is invalid', function() {
+ cy.userLogin();
+ cy.visit(this.addForgeNowUrl);
+ populateForm('bitbucket', 'bitbucket.org', 'test', 'test@example.com', 'on', 'test comment');
+ cy.get('#requestCreateForm').submit();
+
+ cy.get('#swh-input-forge-url')
+ .then(input => {
+ assert.isFalse(input[0].checkValidity());
+ });
+ });
+
});
diff --git a/swh/web/add_forge_now/migrations/0008_turn_request_forge_url_into_url_field.py b/swh/web/add_forge_now/migrations/0008_turn_request_forge_url_into_url_field.py
new file mode 100644
--- /dev/null
+++ b/swh/web/add_forge_now/migrations/0008_turn_request_forge_url_into_url_field.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.28 on 2022-08-16 13:08
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("swh_web_add_forge_now", "0007_rename_denied_request_status"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="request",
+ name="forge_url",
+ field=models.URLField(),
+ ),
+ ]
diff --git a/swh/web/add_forge_now/models.py b/swh/web/add_forge_now/models.py
--- a/swh/web/add_forge_now/models.py
+++ b/swh/web/add_forge_now/models.py
@@ -105,7 +105,7 @@
submitter_forward_username = models.BooleanField(default=False)
# FIXME: shall we do create a user model inside the webapp instead?
forge_type = models.TextField()
- forge_url = models.TextField()
+ forge_url = models.URLField()
forge_contact_email = models.EmailField()
forge_contact_name = models.TextField()
forge_contact_comment = models.TextField(
diff --git a/swh/web/templates/add_forge_now/creation_form.html b/swh/web/templates/add_forge_now/creation_form.html
--- a/swh/web/templates/add_forge_now/creation_form.html
+++ b/swh/web/templates/add_forge_now/creation_form.html
@@ -45,7 +45,7 @@
Forge URL
</label>
<input type="text" class="form-control" id="swh-input-forge-url"
- name="forge_url" required>
+ name="forge_url" oninput="swh.add_forge.validateForgeUrl(this)" required>
<small class="form-text text-muted">
Remote URL of the forge.
</small>
diff --git a/swh/web/tests/add_forge_now/test_migration.py b/swh/web/tests/add_forge_now/test_migration.py
--- a/swh/web/tests/add_forge_now/test_migration.py
+++ b/swh/web/tests/add_forge_now/test_migration.py
@@ -17,6 +17,7 @@
MIGRATION_0005 = "0005_prepare_inbound_email"
MIGRATION_0006 = "0006_request_add_new_fields"
MIGRATION_0007 = "0007_rename_denied_request_status"
+MIGRATION_0008 = "0008_turn_request_forge_url_into_url_field"
def now() -> datetime:
@@ -211,3 +212,41 @@
last_modified_date=datetime.now(timezone.utc),
)
req.clean_fields()
+
+
+def test_add_forge_now_url_validation(migrator):
+
+ state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0007))
+ Request = state.apps.get_model(APP_LABEL, "Request")
+
+ from swh.web.add_forge_now.models import RequestStatus
+
+ request = Request(
+ status=RequestStatus.PENDING.name,
+ submitter_name="dudess",
+ submitter_email="dudess@orga.org",
+ forge_type="cgit",
+ forge_url="foo",
+ forge_contact_email="forge@example.org",
+ forge_contact_name="forge",
+ forge_contact_comment="bar",
+ last_modified_date=datetime.now(timezone.utc),
+ )
+ request.clean_fields()
+
+ state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0008))
+ Request = state.apps.get_model(APP_LABEL, "Request")
+
+ request = Request(
+ status=RequestStatus.PENDING.name,
+ submitter_name="johndoe",
+ submitter_email="johndoe@example.org",
+ forge_type="cgit",
+ forge_url="foobar",
+ forge_contact_email="forge@example.org",
+ forge_contact_name="forge",
+ forge_contact_comment="bar",
+ last_modified_date=datetime.now(timezone.utc),
+ )
+ with pytest.raises(ValidationError, match="Enter a valid URL."):
+ request.clean_fields()
diff --git a/swh/web/tests/api/views/test_add_forge_now.py b/swh/web/tests/api/views/test_add_forge_now.py
--- a/swh/web/tests/api/views/test_add_forge_now.py
+++ b/swh/web/tests/api/views/test_add_forge_now.py
@@ -3,6 +3,7 @@
# License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information
+import copy
import datetime
import threading
import time
@@ -199,6 +200,27 @@
assert len(requests) == 1
+@pytest.mark.django_db(transaction=True, reset_sequences=True)
+def test_add_forge_request_create_invalid_forge_url(api_client, regular_user):
+ api_client.force_login(regular_user)
+ url = reverse("api-1-add-forge-request-create")
+
+ forge_data = copy.deepcopy(ADD_FORGE_DATA_FORGE1)
+ forge_data["forge_url"] = "foo"
+
+ resp = check_api_post_response(
+ api_client,
+ url,
+ data=forge_data,
+ status_code=400,
+ )
+
+ assert resp.data == {
+ "exception": "BadInputExc",
+ "reason": '{"forge_url": ["Enter a valid URL."]}',
+ }
+
+
@pytest.mark.django_db(transaction=True, reset_sequences=True)
def test_add_forge_request_update_anonymous_user(api_client):
url = reverse("api-1-add-forge-request-update", url_args={"id": 1})

File Metadata

Mime Type
text/plain
Expires
Wed, Jul 2, 10:55 AM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
3223685

Event Timeline

About · Developer information · Legal