Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Tests: Recognize callbacks with dots in the Node.js mock server#4764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mgol merged 1 commit intojquery:masterfrommgol:middleware-jsonp-fix
Sep 2, 2020

Conversation

mgol
Copy link
Member

@mgolmgol commentedJul 29, 2020
edited
Loading

Summary

This aligns the Node.js server with the previous PHP one in sendingmock.php
as a callback if there's nocallback 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

Refgh-4754

Checklist

@mgolmgol added this to the3.6.0 milestoneJul 29, 2020
@mgolmgol self-assigned thisJul 29, 2020
@Krinkle
Copy link
Member

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

However, for the local mock proxy that seems fine 👍

@mgolmgol added the Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labelAug 25, 2020
@timmywiltimmywil removed the Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labelAug 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 therequest crashing on that Node.js server and printing a JS error:```TypeError: Cannot read property '1' of null```Refjquerygh-4754
@mgol
Copy link
MemberAuthor

mgol commentedSep 2, 2020
edited
Loading

@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 asmock.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...

@mgolmgol merged commitdf6858d intojquery:masterSep 2, 2020
@mgolmgol deleted the middleware-jsonp-fix branchSeptember 2, 2020 16:42
mgol added a commit to mgol/jquery that referenced this pull requestSep 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 istriggered by a recently added test. This prevents the request crashing on thatNode.js server and printing a JS error:```TypeError: Cannot read property '1' of null```Closesjquerygh-4764Refjquerygh-4754(cherry picked from commitdf6858d)
@mgol
Copy link
MemberAuthor

mgol commentedSep 2, 2020

Landed onmaster indf6858d and on3.x-stable in4c572a7.

mgol added a commit to mgol/jquery that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.Refjquerygh-4764
mgol added a commit that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.Refgh-4764Closesgh-4871
mgol added a commit that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.Refgh-4764Closesgh-4871(cherry picked froma702746)
mgol added a commit to mgol/jquery that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.This is a 1.x/2.x version of pRjquerygh-4871Refjquerygh-4764Refjquerygh-4871
mgol added a commit to mgol/jquery that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.This is a 1.x/2.x version of PRjquerygh-4871.The change doesn't require a release; it's meant at installations testingthe latest state of `1.12-stable` & `2.2-stable` branches.Refjquerygh-4764Refjquerygh-4871
mgol added a commit to mgol/jquery that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.This is a 1.x/2.x version of PRjquerygh-4871.The change doesn't require a release; it's meant at installations testingthe latest state of `1.12-stable` & `2.2-stable` branches.This change also fixes testing on Travis & on Chrome/Firefox.Refjquerygh-4764Refjquerygh-4871
mgol added a commit that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.This is a 1.x/2.x version of PRgh-4871.The change doesn't require a release; it's meant at installations testingthe latest state of `1.12-stable` & `2.2-stable` branches.This change also fixes testing on Travis & on Chrome/Firefox.Closesgh-4875Refgh-4764Refgh-4871
mgol added a commit to mgol/jquery that referenced this pull requestApr 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 happensoften enough that the whole jQuery repository directory structure is deployedonto the server with PHP enabled that it makes is easy to introduce securityissues if this cleanup is not done.This is a 1.x/2.x version of PRjquerygh-4871.The change doesn't require a release; it's meant at installations testingthe latest state of `1.12-stable` & `2.2-stable` branches.This change also fixes testing on Travis & on Chrome/Firefox.Closesjquerygh-4875Refjquerygh-4764Refjquerygh-4871(cherry picked fromacb7c49)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@KrinkleKrinkleKrinkle approved these changes

@gibson042gibson042gibson042 approved these changes

Assignees

@mgolmgol

Labels
Milestone
3.6.0
Development

Successfully merging this pull request may close these issues.

4 participants
@mgol@Krinkle@timmywil@gibson042

[8]ページ先頭

©2009-2025 Movatter.jp