Giter VIP home page Giter VIP logo

Comments (27)

mpscholten avatar mpscholten commented on July 20, 2024

I think the first thing which should be done is to add namespacing and autoloading.
But what should the vendor-namespace be? PhpSecLib or Phpseclib?
And then we should move the phpseclibdirectory into a src directory.

Should Crypt_Base be an abstract class? Yes I think this would be a good decision 👍

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

Whether the directory is called phpseclib or src does not matter at all, so I'd just leave it as is.

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

Using an extra PHP5 repository is going to be a huge pain when it comes to github tickets. Stuff reported against phpseclib can not be easily moved to phpseclib-php5 etc.

Also, development with two independent repositories is just not going to work (at all).

I would like to stress my opinion of the only thing that is going to work well:

  • Single repository
  • Branches (master branch with php4, develop branch with php5 and new features)
  • After merging a patch into the master branch, the master branch MUST be merged into develop immediately, usually by the same person (who reviewed and understood the original changes and can resolve conflicts if necessary.
  • Bugfixes for bugs present in the master branch SHOULD go into the master branch first, if possible.
  • New features SHOULD be added to the develop branch, so that new language features can be used.
  • Once the develop branch is deemed stable, the master branch is merged into an oldstable branch and develop is merged into master.

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

Developer access to different repositories is nice to have but not necessary. If people commit crap, you just slap them and if they don't behave you kick them and if necessary revert. Also, it should be required that everything MUST go through a review process anyway.

from phpseclib-php5.

JonTheNiceGuy avatar JonTheNiceGuy commented on July 20, 2024

Why not enforce all changes to be done in a personal fork, and then every request requires a pull request?

This lets you separate the change from the approval, and you can see who has approved everything.

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

@JonTheNiceGuy Yes, I would like to see that (or something similar). Nevertheless, developers need write access to the repository as someone has to do the merging.

from phpseclib-php5.

mpscholten avatar mpscholten commented on July 20, 2024

👍

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

After giving this some thought... I'm willing to try it out with a few changes.

Instead of the php5 branch being called develop I think it ought to be called php5.

Also, I think new features should be added to the master branch unless the feature can only be implemented in PHP5 and is absolutely essential.

Like with the SFTP stream wrapper I define a class variable as static to re-use SSH connections. Whether or not that's essential is debatable, I suppose, but given how easily I can come up with use-cases where that'd be the expected behavior I'm leaning towards saying it is.

And even that one, PHP5-only though it is, is in master.

Really, in my mind, the php5 branch should chiefly be for system-wide changes. phpseclib does not need to use namespacing. It might be nice but SSH and X.509 and elliptical curves and whatever else can be implemented without namespaces. That syntactic sugar is what the php5 branch is for.

And, imho, php5-only code can go in master, with the caveat that it should only go in master if it's absolutely necessary. And even then namespaces shouldn't be used in master. The API's should be consistent. Everything namespace'd in the php5 branch, nothing namespace'd in the master branch.

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

One idea of adding new features to the development branch only is forcing people to upgrade their PHP installations. Another is that you can use proper object-orientation as in modern PHP projects and do not have to worry about all the weirdnesses that PHP4 might do to objects.

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

The problem I have with forcing people to upgrade their PHP installations is that phpseclib's first major bullet point on it's homepage is "Compatibility". phpseclib (in particular it's SSH implementation) was originally written to be usable everywhere libssh2 wasn't. That it's also superior in most other ways is a nice little bonus but that wasn't the intent going into it.

Like if you want to be doing BigInteger stuff right you shouldn't be using phpseclib's BigInteger class at all - you should use gmp. But phpseclib isn't about doing stuff right - it's about working everywhere it can. And imho that's a pretty big part of phpseclib's branding - the fact that it works everywhere.

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

Todays world looks like the following to me: PHP5 everywhere, GMP usually not installed. So not relying on GMP is a good idea if you just want it to work, but not making use of PHP5's features is just unnecessarily painful.

Also, nobody tests new features on PHP4 because nobody is using it anyway. Not even for development and the test suite wouldn't run on PHP 4 either. It's been dead for too long.

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

I don't actively test on PHP4 myself either lol. I develop code that I expect will work on PHP4 but do all my testing with PHP 5.5. I mean, so long as you follow a few simple rules of thumb, it's not that hard to write code that /should/ work on PHP4. ie. don't do public / private (which I don't like anyway), don't use __construct, etc.

Like with phpseclib/phpseclib@22502c2 / phpseclib/phpseclib#154 (comment). I didn't test the commit prior to that on any version of PHP lower than 5.5. And when it broke on PHP 5.3 someone filed a bug report and it was fixed. Really, I think that's the best we can do. Well, short of unit tests, I guess, but even then, it looks like travis-ci.org is running their 5.3 unit tests on 5.3.3 - not on 5.3.6 or 5.3.7 or whatever.

from phpseclib-php5.

phpsyscoder avatar phpsyscoder commented on July 20, 2024

Hi there all (and sorry for my long absence... as usual much work everytime)

i think i followed most of the discussions about namespacing, autoloading, __MAGIC's() and other php5 feature requests.

Imho phpseclib, it self, does not need all this features but maybe some projects where phpseclib would be used and where ie php5 autoloading or strict namespacing is required by desing for all libs.

Using more of this php5 features means also more dealing with the traps comming with those... like the trap of the __destructor() behavior we had some months ago.

Then one question also comes in my mind: Which php5 compatibility should be used? 5.0? 5.1? 5.2? Namespacing was, if i remember it right, available since 5.3.?

Maintain a stable, fast, php modules independent and high php-version compatible crypt code is in it self a high demand.

Maintain 2 branches (php4 php5), even with Bantu's suggestions, will make it additional harder.
OK, this could be avoid if we give up php4 compatibility copmlete and define a new compatibility for a given php 5 version, ie 5.3.3

All this together... my opinion is that phpseclib not need php5.x? features :-) Will make the project unnecessary harder to maintain, dealing with more traps, for not much benefit.

Projects want using phpseclib could, imho, still using phpseclib without Namespacing/Autoloading/Catch-Trow-Error etc... with small or in most cases no modifications of their code.

Or did i miss something?

I just not see a crucial need of a php5 branch.
Maybe for throw -> error catch()'ing... yes. For security reasons. Because using user_error() is imho insecure because it possible will disclosure system paths to the public.

Greetings :-)

btw: The cipher classes are all well testet with php4... seem's i'm the only one haha :-)

from phpseclib-php5.

JonTheNiceGuy avatar JonTheNiceGuy commented on July 20, 2024

Please remember that PHP4.4 (the last release of the PHP4 tree) went EOL in 2008... Nearly 2000 days ago! PHP 5.2 went EOL in January 2011, over 1000 days ago. This means there will be no security patches for these versions. I understand a desire for compatibility, but realistically, these versions should be dead, so please focus on the current versions and only backport important fixes to older trees.

from phpseclib-php5.

phpsyscoder avatar phpsyscoder commented on July 20, 2024

Jon, you mention absolut valid points.

My point was why using for example Namespace when not really necessary but would break compatibility? The same for other compat.breakers... or possible "trap'ers".

Maybe i'm allone with this opinion... :-) but does phpseclib, or projects using phpseclib, really need it?

Besides the < php5.3 compat issue with the Namespace... using it also would break current projects using phpseclib... because ie "new Crypt_AES()" does'nt exists then anymore.

So is there really a need of it?

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

Some projects distinguish themselves from other projects by embracing the latest and greatest in PHP. By enabling developers to do things the "right way", without regard for BC.

Consider frameworks. They're a dime a dozen and there's a framework for every niche. There's a framework for the PHP4.4 niche and there's a framework for the PHP: The Right Way niche. There's are frameworks for people who want to use DAO's, frameworks for people who prefer ORM, etc, and if someone wants to corner one of those niches they write a framework to provide the features that that niche wants.

Another example. I used to develop phpBB modifications. There was already a "Birthday hack" out there but I wrote my own to cater to the niche of people who wanted to get modifications from phpbb.com and not phpbbhacks.com. Obviously that wasn't everyone but my project found it's niche and flourished within it.

phpseclib's niche has never been "the right way" audience. It's always been more of a last mile solution for people for whom no other solution works, in my mind. That it's not the niche some of ya'll prefer doesn't mean it's not a valid niche.

Really, I think the core of the problem we're facing here is that one niche wants to replace usurp the other niche for this particular project and the "PHP: The Right Way" niche has always been a bit more vocal and outspoken than the PHP4-niche. And all of this would be a non-issue if someone from the "PHP: The Right Way" niche just developed their own SSH implementation, but SSH from scratch is pretty darned hard, so here we are.

Ultimately, I think both niches can be accommodated. The phpseclib-php5 repo was an attempt to do so. Maybe a php5 branch is a better attempt idk.

I really don't see developing code that should work on PHP4 as being that laborious a task. And the merge conflicts that will occur are gonna be more of a one-time thing too. Maybe we could have a git hook auto-translate things over and fix things when unit tests prove that they don't work.

from phpseclib-php5.

JonTheNiceGuy avatar JonTheNiceGuy commented on July 20, 2024

How about this:

  1. Park the existing branch with a tag of "PHP4 and LEQ-PHP5.2" and only provide security fixes to those as they appear.
  2. All new code aims to support PHP5.3.
  3. Anything PHP5.2 or lower uses the PHP4 version.

This then means that if you need to fix something which is pre-PHP5.3 then you still can - the tag exists and can be improved if needed, but all future code should aim to support at least the oldest-still-supported version. Again, as versions of PHP are EOL'd, you can park the branches in the same way, and move to the next still supported version.

Regarding the "audience" in question, I think it's anyone who needs SSH for PHP, but can't use the LibSSH2 PECL extension. Saying it's "not for" someone limits adoption.

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

Saying it's "not for" someone limits adoption.

The "PHP: The Right Way" niche might not want to use code intended for the pre-PHP5.3 niche but they can.
The pre-PHP5.3 niche, however, cannot use code intended for the "PHP: The Right Way" niche, regardless of how much they might want to.

  1. All new code aims to support PHP5.3.

Making PHP5.3 code work on PHP4 really isn't that difficult. Remove name spaces, rename __construct to whatever the class name is, etc. If such little effort can get phpseclib ten more people using it then why not do it?

If you want to write PHP5.3 fine. Just make it so it's PHP4 compatible before submitting the pull request to master and viola. If it's a worthwhile enough feature I might even be willing to do it myself. I mean, I don't want to encourage laziness in developers by doing all the manual labor myself but if it's a sufficiently useful feature I might be willing to.

And as I've stated, just because you write code that should work on PHP4 doesn't mean you actually have to test it on PHP4. I test all my code on PHP5.5. For benchmarking purposes I do have PHP4.4 on my system but that doesn't mean I use it on a day-to-day basis.

from phpseclib-php5.

mpscholten avatar mpscholten commented on July 20, 2024
  1. All new code aims to support PHP5.3.

👍 Seems like a solution for me. I think PHP4 is death, a quick search for php4 usage stats shows that php4 is just used by a minority.

If you want to write PHP5.3 fine. Just make it so it's PHP4 compatible before submitting the pull request to master and viola.

I don't see why? Why not stop development of the php4 version here? everyone using this lib with an old pre-php5.3 project can use that version, everyone else can use the new version.

from phpseclib-php5.

salmonila avatar salmonila commented on July 20, 2024

Another project that's having this same debate:

leafo/lessphp#390

from phpseclib-php5.

bantu avatar bantu commented on July 20, 2024

The lack of namespacing is somewhat causing issues in practise. See phpseclib/phpseclib#125

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

Breaking BC with itself will somewhat cause issues in practice too.

And in any event, this repo was an attempt to respond to the namespacing issue. And as a further concession I was willing to do a php5 branch in the phpseclib repo instead. petrich expressed concerns about this and I'll admit I have some as well but I'm willing to try it. If it doesn't work out it doesn't work out but at least we can say we tried it out. But I guess this discussion has stalled out too.

I think PHP4 is death, a quick search for php4 usage stats shows that php4 is just used by a minority.

The law of diminishing marginal returns suggest that each successive new feature is going to add increasingly less and less.

I recently added subsystem support to Net/SSH2 and in the seven or so years phpseclib has been out only recently has someone asked for subsystem support. And I implemented it because I go that extra mile to make phpseclib work for everyone. I could have been like "nope - this feature is only going to benefit one person so I won't do it" but I didn't. And even if PHP4 support for the master branch only benefited just one person that's enough for me. Although I will concede that that philosophy gets a little murky when features are incompatible.

That there'd be conflicts with PEAR was a known issue from the get go. That's been an issue for seven years. It's unfortunate it's an issue and will continue to be an issue with master but I'm not willing to trade one issue for another. And in any event, in all the years phpseclib has been around, only once has that issue been raised. And that's just as much an issue for PEAR as it is for phpseclib and although new PEAR contributions might need to be namespaced (I don't know) old ones aren't being retroactively namespace'd out of respect for BC.

from phpseclib-php5.

mpscholten avatar mpscholten commented on July 20, 2024

Okay, let's start with creating a new php5 branch

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

I'll pull within the next few days your work into a new php5 branch.

I was finally let back into my apartment after my recent flooding and am trying to put everything back together. Finding lots of broken furniture and other damaged stuff :(

from phpseclib-php5.

mpscholten avatar mpscholten commented on July 20, 2024

Oh, I'm sorry :(

from phpseclib-php5.

terrafrost avatar terrafrost commented on July 20, 2024

What are your thoughts on this btw?:

phpseclib/phpseclib#163 (comment)

from phpseclib-php5.

phpsyscoder avatar phpsyscoder commented on July 20, 2024

flooding

I'm also sorry Terra

from phpseclib-php5.

Related Issues (1)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.