Pierwszy skrypt w OOP Opcje

[SN@JPER^]

Witam, mój pierwszy skrypt, klasa...

Dobrze załapałem póki co programowanie obiektowe? Jakie błędy robie? Co poprawić?

[PHP] pobierz, plaintext 
class UploadFiles{
    private $file;
    private $dir;
    private $mime = array('image/jpeg', 'image/gif', 'image/png');
    private $chmod=777;
 
    public function  saveFile($fileName, $dir) {
        $this->file=$_FILES[$fileName];
        $this->dir=$dir.'/'.$this->file['name'];
 
        if(isset($_FILES[$fileName])){
 
            if($this->file['error']==0){
                if(is_uploaded_file($this->file['tmp_name'])){
                    if(in_array($this->file['type'], $this->mime)){
                        if(move_uploaded_file($this->file['tmp_name'], $this->dir)===true){
                            chmod($this->dir, $this->chmod);
                            return true;
                        }
                    }else{
                        //Nieprawidłowy format pliku
                    }
                }
            }else{
                switch($_FILES[$this->file]['error']){
                    case 1: /* KOMUNIKAT o przekroczeniu wartości upload_max_filesize*/; break;
                    case 2: /* KOMUNIKAT o przekroczeniu wartości max_filesize*/; break;
                    case 3: /* KOMUNIKAT o wysłanym częściowo pliku*/; break;
                    case 4: /* KOMUNIKAT o nie wysłaniu żadnego pliku*/; break;
                }
            }
        }else{
            return false;
        }
    }
 
}
[PHP] pobierz, plaintext 

[Crozin]

Źle...
1) Cały obiekt składa się wyłącznie z jednej metody, co za tym idzie nie ma sensu korzystać z pól/własności/składowych/jakkolwiek by tego nie nazwać klasy, skoro używane jest to jedynie w kontekście lokalnym
2) Takie sprawdzanie typu mime to o kant d..y rozbić można. _FILES[abc][type] jest ustawiane przez użytkownika, czyli może tam być cokolwiek
3) W PHP istnieją przedefiniowane stałe określające kod błędu
4) Masz takie coś jak wyjątki... dużo lepiej się sprawdza niż zwykłe true/false
5) Nazwa klasy też niezbyt dobrze trafiona.

[darko]

Dobrze jest, jak na pierwszy skrypt. Nie ma sensu usuwać pól, jak dojdzie jeszcze jakaś metoda, to wtedy się przydadzą. Z tą obsługą błędów to fakt, mógłbyś lepiej to rozwiązać. Właściwie mógłbyś zwracać rezultat move_uploaded_file, przynajmniej byłoby wiadomo czy uploadowano.

[SN@JPER^]

Będzie m.in metoda do nazw plików.

2) Takie sprawdzanie typu mime to o kant d..y rozbić można. _FILES[abc][type] jest ustawiane przez użytkownika, czyli może tam być cokolwiek

Co proponujesz?

[Crozin]

Co proponujesz?

Typ MIME wyciągnąć z pliku dopiero po stronie serwera - a potem (tj.: fragment z in_array()) już tak samo.

[SN@JPER^]

W takich typu klasach, lepiej użyć __construct czy tak jak w moim przypadku?

Typ MIME wyciągnąć z pliku dopiero po stronie serwera - a potem (tj.: fragment z in_array()) już tak samo.

Czyli po move_uploded_file sprawdzic mime? Jeżeli nie bedzie nieprawidłowy to co wtedy? usunać plik?

Ten post edytował SN@JPER^ 10.01.2010, 22:48:10

[Crozin]

Tak, jeżeli z plikiem jest coś nie tak to usuń go.

[SN@JPER^]

Próbuje użyć funkcji mime_content_type, ale coś nie działa...

na localhost default nie działa, musiałem odznaczyć ";" w extension=php_mime_magic.dll

Po odznaczeniu nic nie wyświetla.

[marcio]

TO sprawdzaj rozszerzenie i tyle.

[zegarek84]

Próbuje użyć funkcji mime_content_type, ale coś nie działa...

którą masz wersję php?? - czytaj może co piszą w manualu - te komunikaty:
mime_content_type - a dalej to masz link jakiej funkcji powinieneś użyć jeśli masz php 5.3
finfo_file

[SN@JPER^]

No to sie zgadza. mam wersje "PHP Version 5.2.5".

co do tego mime_type, wydaje sie jednak, że sprawdzanie rozszerzenia jest lepszym pomysłem. Co o tym sądzicie?

Troche mnie martwi fakt, ze najpeirw musi z uploadowac plik, sprawdzic mime, jeżeli sie nie zgadza to go usunac(!).

a i jeszcze "5) Nazwa klasy też niezbyt dobrze trafiona." - czemuż to?

[marcio]

co do tego mime_type, wydaje sie jednak, że sprawdzanie rozszerzenia jest lepszym pomysłem. Co o tym sądzicie?

Troche mnie martwi fakt, ze najpeirw musi z uploadowac plik, sprawdzic mime, jeżeli sie nie zgadza to go usunac(!).

Bo jest to bardziej logiczne i szybsze tym bardziej ze sam sie juz kiedys przejechalem na sprawdzaniu mime plikow bo nie wiedzialem ze poprzez naglowki mozna je zmodyfikowac, i staje sie to "bezpieczne" dopiero wtedy gdy go sprawdzimy jak plik jest juz na srv.

[SN@JPER^]

a no chyba, że tak

[CapaciousCore]

A nie lepiej recznie sprawdzac naglowek pliku?

[SN@JPER^]

A nie lepiej recznie sprawdzac naglowek pliku?

Jak?

Dobra idea tego sprawdzania rozszerzenia?

[PHP] pobierz, plaintext 
class Upload{
    private $dir;
    private $type = array('jpg', 'gif', 'png');
    private $chmod = 0777;
    private $file;
 
    public function __construct($fileName, $dir){
        $this->file = $_FILES[$fileName];
        $this->dir = $dir;
 
        if(isset($this->file)){
            $nameFile = $this->createTitle($this->file['name']);
            if(!file_exists($this->dir.'/'.$nameFile)){
                if(is_uploaded_file($this->file['tmp_name'])){
 
                    if(move_uploaded_file($this->file['tmp_name'], $this->dir.'/'.$nameFile)){
                        chmod($this->dir.'/'.$this->file['name'], $this->chmod);
 
                        $d='';
 
                        $d = opendir($dir);
                        while(false !== ($f = readdir($d))) {
                            if($f == $this->file['name']){
                                $ex = explode('.', $f);
                                if(in_array($ex[1], $this->type)){
                                    //wgrano
                                }else{
                                    if(unlink($this->dir.'/'.$this->file['name'])){
                                        echo 'Zły format pliku!';
                                    }
                                }
                            }
                        }
                        closedir($d);
                    }
                }
            }else{
                echo 'Taki plik już istnieje!';
            }
        }
    }
 
    private function createTitle($title){
        $title = strtolower($title);
        $title = str_replace(' ', $this->replacechar, $title);
        return $title;
    }
}
[PHP] pobierz, plaintext 

Ten post edytował SN@JPER^ 5.03.2010, 17:23:45

[Cysiaczek]

Czy możesz napisać, co zamierzałeś zrobić, czyli po co Ci był potrzebny kod? Inaczej mówiąc - co kod miał robić (nie co robi teraz). Własnymi słowami.

[SN@JPER^]

Jak widać u góry dyskutowaliśmy jak sprawdzić rozszerzenie/mimetype. Wielu proponowało, aby sprawdzić rozszerzenie/mimetype po wgraniu pliku na serwer.

Dobrze to rozwiązałem?

[Cysiaczek]

Nie zrozumiałeś mnie

Witam, mój pierwszy skrypt, klasa...

Dobrze załapałem póki co programowanie obiektowe? Jakie błędy robie? Co poprawić?

Ja bym chciał, abyś opisał to, co myślałeś zanim zacząłeś pisać kod, który zaprezentowałeś wyżej, bo to istotne.

[SN@JPER^]

Prosty skrypt uploadu obrazów na serwer... Sprawdzanie rozszerzenie/mime etc.

[phpion]

[PHP] pobierz, plaintext 
$ex = explode('.', $f);
 
if(in_array($ex[1], $this->type)){
  //wgrano
}
[PHP] pobierz, plaintext 

Zabezpieczenie żadne. Przepuści np. plik moje_zdjecia_porno.jpg.exe.