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

CAPTCHA doesn't work without GD #4361

Closed
Arantor opened this issue Oct 17, 2017 · 12 comments
Closed

CAPTCHA doesn't work without GD #4361

Arantor opened this issue Oct 17, 2017 · 12 comments
Assignees
Milestone

Comments

@Arantor
Copy link
Contributor

Arantor commented Oct 17, 2017

If GD is not present - and it's never indicated that it is mandatory - the image CAPTCHA fails to load the pure image fallbacks that are present.

Either the GD library should be made clear to be a dependency like mbstring is, or the fallback should work.

@Arantor
Copy link
Contributor Author

Arantor commented Oct 17, 2017

Specifically: the check for fonts looks for a folder that contains the font images having the same name as a .gdf file in the fonts folder. Given that the font might be called AnonymousPro and the folder be 'anon', this isn't going to work out.

Additionally, anon has mixed case filenames, which is also bad.

@sbulen
Copy link
Contributor

sbulen commented Nov 23, 2017

It would be easy enough to check for GD support on install & upgrade.

Based on prior discussion (#4148), I thought GD library was standard with php installs. I could never find a doc explicitly stating that, though. So maybe it is safest to add a test.

Would a check for a specific font name also be appropriate? I'm not sure what to do about potential font issues. Should that be an install/upgrade test, or a test upon enabling the feature?

@Arantor
Copy link
Contributor Author

Arantor commented Nov 23, 2017

We found out about it on the basis that we were doing tests using Cloud9 which doesn't ship any of the PHP extensions by default (as it's based on Debian). Surprising what breaks under that situation.

In the case of the fonts - there's several fonts of font fallback images, which could be used but fail to be used because the names don't match what is expected. This list of viable replacements could be determined by what files exist or could be determined by a preset hardcoded list. Either works.

@sbulen
Copy link
Contributor

sbulen commented Nov 24, 2017

As an FYI - GD is listed as a recommendation in 2.0.x:
https://wiki.simplemachines.org/smf/SMF2.0:Requirements_and_recommendations

Still, I agree the edit is smart to do.

I'm still thinking about the font. What I've read indicates GD has a set of 'default' fonts with simple stupid names, "1" "2" "3" "4" and "5" that correspond to different sizes. If we use the default font, that may sidestep any potential issues with a font not found.

@Arantor
Copy link
Contributor Author

Arantor commented Nov 24, 2017

That's not the problem.

If you don't have GD, you can't use GD's fonts - so you have to use the image fallbacks that are given - look in the theme folder, you'll see a fonts folder in which are folders containing pre-made images for the CAPTCHA. The problem is that SMF in its current code cannot find those folders and thus those images.

There isn't a problem if GD is enabled, which is what everyone's been testing with.

@sbulen
Copy link
Contributor

sbulen commented Nov 24, 2017

Ah, thanks... So if we make GD a requirement, there is no issue.

I'm leaning towards making GD a requirement, myself... It is implemented pretty broadly.

(Those builds with NO addons like Cloud9 strike me as a special form of masochism...)

@illori
Copy link
Contributor

illori commented Nov 25, 2017

i thought when i changed the font for the CAPTCHA i created the images with the correct names so they would work. i believe i had tested with GD disabled but i cant recall.

@MissAllSunday MissAllSunday added this to the Final milestone Sep 4, 2019
@MissAllSunday MissAllSunday self-assigned this Sep 4, 2019
@sbulen
Copy link
Contributor

sbulen commented Mar 15, 2021

@MissAllSunday is this really a RC4 requirement?

Please remove the label. OR... Let's discuss a specific change needed...

@MissAllSunday
Copy link
Contributor

As it can be seen on the issue's history, the original label for the issue was Final, that was back when there wasn't an RC4 label created yet.

Fell free to re-label it for the original target which was Final.

@sbulen
Copy link
Contributor

sbulen commented Mar 15, 2021

I think we should remove the label.

This shouldn't hold up either rc4 or final.

It appears to be an unusual edge condition for certain minimalistic builds.

@MissAllSunday
Copy link
Contributor

Indeed, our priorities have changed and we must adjust the current opened issues to reflect that.

@MissAllSunday MissAllSunday removed this from the RC4 milestone Mar 15, 2021
@Sesquipedalian Sesquipedalian modified the milestones: The future, 2.1.1 Jul 29, 2021
@Sesquipedalian
Copy link
Member

After much suffering to test this, I cannot reproduce it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants