🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@mgol
Copy link
Member

@mgol mgol commented Jul 29, 2020

Summary

This aligns the Node.js server with the previous PHP one in sending mock.php
as a callback if there's no callback parameter in the query string which is
triggered by a recently added test. This prevents the request crashing on that
Node.js server and printing a JS error:

TypeError: Cannot read property '1' of null

Ref gh-4754

Checklist

@mgol mgol added this to the 3.6.0 milestone Jul 29, 2020
@mgol mgol self-assigned this Jul 29, 2020
@Krinkle
Copy link
Member

Krinkle commented Aug 17, 2020

This is usually recommended against because it allows things like document.<something>, e.g. document.body.lastChild.firstChild.click or some such.

However, for the local mock proxy that seems fine 👍

@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Aug 25, 2020
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Aug 31, 2020
@timmywil
Copy link
Member

We're going to check why the filename is used as the callback name.

This aligns the Node.js server with the previous PHP one in accepting `mock.php`
as a callback which is triggered by a recently added test. This prevents the
request crashing on that Node.js server and printing a JS error:
```
TypeError: Cannot read property '1' of null
```

Ref jquerygh-4754
@mgol mgol force-pushed the middleware-jsonp-fix branch from 446c05c to 2f1050d Compare September 2, 2020 16:22
@mgol
Copy link
Member Author

mgol commented Sep 2, 2020

@timmywil @Krinkle I've just had another look. Code I'm changing doesn't check the query parameter but a REST-like path. Since we only serve it as mock.php, that's the only path you can get via this method so a dot seems safe.

On the other hand, we do not validate the callback parameter value in any way either in the PHP server or in the Node.js one, e.g. the following URL:
http://builds.jenkins.jquery.com/jquery/c18dc49699bc27481a4af36ed1a0ee1b19c6eb03/test/data/mock.php?action=jsonp&callback=document.body.innerHTML=
returns:

document.body.innerHTML=({ "data": {"lang": "en", "length": 25} })

Therefore, this PR should be safe to land as-is; we can address callback validation separately if needed. But, since this was the behavior for ages, this shouldn't be a huge issue in practice...

@mgol mgol removed the Needs review label Sep 2, 2020
@mgol mgol merged commit df6858d into jquery:master Sep 2, 2020
@mgol mgol deleted the middleware-jsonp-fix branch September 2, 2020 16:42
mgol added a commit to mgol/jquery that referenced this pull request Sep 2, 2020
This aligns the Node.js server with the previous PHP one in sending `mock.php`
as a callback if there's no `callback` parameter in the query string which is
triggered by a recently added test. This prevents the request crashing on that
Node.js server and printing a JS error:
```
TypeError: Cannot read property '1' of null
```

Closes jquerygh-4764
Ref jquerygh-4754

(cherry picked from commit df6858d)
@mgol
Copy link
Member Author

mgol commented Sep 2, 2020

Landed on master in df6858d and on 3.x-stable in 4c572a7.

mgol added a commit to mgol/jquery that referenced this pull request Apr 9, 2021
Only allow alphanumeric characters & underscores for callback parameters.
The change is done both for the PHP server as well as the Node.js-based version.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

Ref jquerygh-4764
mgol added a commit that referenced this pull request Apr 13, 2021
Only allow alphanumeric characters & underscores for callback parameters.
The change is done both for the PHP server as well as the Node.js-based version.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

Ref gh-4764
Closes gh-4871
mgol added a commit that referenced this pull request Apr 13, 2021
Only allow alphanumeric characters & underscores for callback parameters.
The change is done both for the PHP server as well as the Node.js-based version.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

Ref gh-4764
Closes gh-4871

(cherry picked from a702746)
mgol added a commit to mgol/jquery that referenced this pull request Apr 16, 2021
Only allow alphanumeric characters & underscores for callback parameters.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

This is a 1.x/2.x version of pR jquerygh-4871

Ref jquerygh-4764
Ref jquerygh-4871
mgol added a commit to mgol/jquery that referenced this pull request Apr 16, 2021
Only allow alphanumeric characters & underscores for callback parameters.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

This is a 1.x/2.x version of PR jquerygh-4871.

The change doesn't require a release; it's meant at installations testing
the latest state of `1.12-stable` & `2.2-stable` branches.

Ref jquerygh-4764
Ref jquerygh-4871
mgol added a commit to mgol/jquery that referenced this pull request Apr 16, 2021
Only allow alphanumeric characters & underscores for callback parameters.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

This is a 1.x/2.x version of PR jquerygh-4871.

The change doesn't require a release; it's meant at installations testing
the latest state of `1.12-stable` & `2.2-stable` branches.

This change also fixes testing on Travis & on Chrome/Firefox.

Ref jquerygh-4764
Ref jquerygh-4871
mgol added a commit that referenced this pull request Apr 29, 2021
Only allow alphanumeric characters & underscores for callback parameters.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

This is a 1.x/2.x version of PR gh-4871.

The change doesn't require a release; it's meant at installations testing
the latest state of `1.12-stable` & `2.2-stable` branches.

This change also fixes testing on Travis & on Chrome/Firefox.

Closes gh-4875
Ref gh-4764
Ref gh-4871
mgol added a commit to mgol/jquery that referenced this pull request Apr 29, 2021
Only allow alphanumeric characters & underscores for callback parameters.
This is only test code so we're not fixing any security issue but it happens
often enough that the whole jQuery repository directory structure is deployed
onto the server with PHP enabled that it makes is easy to introduce security
issues if this cleanup is not done.

This is a 1.x/2.x version of PR jquerygh-4871.

The change doesn't require a release; it's meant at installations testing
the latest state of `1.12-stable` & `2.2-stable` branches.

This change also fixes testing on Travis & on Chrome/Firefox.

Closes jquerygh-4875
Ref jquerygh-4764
Ref jquerygh-4871

(cherry picked from acb7c49)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants