Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-7818: Fixed direct acces to index.php with long URL #70

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

OLTC-fperrin
Copy link
Contributor

@OLTC-fperrin OLTC-fperrin commented Feb 20, 2024

Question Answer
JIRA issue IBX-7818
Type improvement
Target Ibexa version v4.6
BC breaks no

Fixed an issue when index.php can be accessed directly if the URL is long and complexe enough

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (not sure which I should target since it's been in there for a very long time).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs). (N/A)
  • Asked for a review.

@ibexa/engineering can you review this PR ? thank you

@adamwojs
Copy link
Member

@glye Could you please take a look on this PR?

Copy link
Contributor

@glye glye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OLTC-fperrin Ye gods, it looks like you're right! Considering that it is non-security, target branch should be v4.6. Technically v3.3 is maintained until Sunday, but we're unlikely to get this past QA by then.

@glye
Copy link
Contributor

glye commented Jun 27, 2024

@OLTC-fperrin Correction, it seems we'll get another v3.3 release out, so please set target branch to v3.3.

@OLTC-fperrin
Copy link
Contributor Author

Hello @glye,

so please set target branch to v3.3.

Not entirely sure how I do that, I don't see a 3.3 branch in the list.

@glye
Copy link
Contributor

glye commented Jul 8, 2024

My mistake. @webhdx Is the 1.0 branch here used for v3.3, or is that a different repo?

@webhdx
Copy link
Contributor

webhdx commented Jul 8, 2024

@glye that's correct. '1.0' branch is used for Ibexa DXP 3.3 version.

@glye
Copy link
Contributor

glye commented Jul 8, 2024

Thanks. So @OLTC-fperrin please set the target branch to 1.0. Thanks!

@OLTC-fperrin OLTC-fperrin changed the base branch from main to 1.0 July 8, 2024 08:14
@OLTC-fperrin
Copy link
Contributor Author

@glye , ok, it's done from github, but i'm not sure that was the correct way (130 commits ??), should I rebase my commit on the branch 1.0 ?

anyway, if it's easier, feel free to just close this PR and add the fix to the branch you want properly.

@glye
Copy link
Contributor

glye commented Jul 8, 2024

Rebase, yes please.

@OLTC-fperrin OLTC-fperrin changed the base branch from 1.0 to main July 8, 2024 08:21
@OLTC-fperrin
Copy link
Contributor Author

@glye ok, i'm thoroughly confused, the template files don't exist on branch 1.0. Not sure what you want me to do.

@glye
Copy link
Contributor

glye commented Jul 8, 2024

Come to think of it, I copied the templates here from the docker repo last year, because they were not maintained in that repo: #50

@webhdx What would you recommend? Should we just target v4.6, then?

@webhdx
Copy link
Contributor

webhdx commented Jul 8, 2024

Yes let's keep it for 4.6 and main branches. I've looked into the documentation and it links the file on main branch.

@webhdx
Copy link
Contributor

webhdx commented Jul 9, 2024

@OLTC-fperrin can you rebase your PR against 4.6 branch? As soon as it's done I'll pass your PR to our QA team.

@webhdx webhdx requested a review from a team July 9, 2024 08:21
@OLTC-fperrin OLTC-fperrin changed the base branch from main to 4.6 July 9, 2024 16:18
@OLTC-fperrin
Copy link
Contributor Author

OLTC-fperrin commented Jul 9, 2024

Ok I rebased and changed the target branch of the PR, I hope I did that correctly 🤞

@glye
Copy link
Contributor

glye commented Jul 9, 2024

Many thanks, looks good to me!

@konradoboza
Copy link
Contributor

@OLTC-fperrin thank you for your contribution! I passed this PR to our QA team on your behalf to be tested. Once we have confirmation all works correctly, we will merge it to 4.6 and up to main branch.

Copy link

@KamilSznajdrowicz KamilSznajdrowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA Approved
Regression: ibexa/commerce#1065

@konradoboza konradoboza merged commit 1bf4267 into ibexa:4.6 Oct 8, 2024
11 of 12 checks passed
@konradoboza
Copy link
Contributor

@OLTC-fperrin thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working QA approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants